prowgen: use preset for registry pull credentials volume#4994
prowgen: use preset for registry pull credentials volume#4994petr-muller wants to merge 2 commits intoopenshift:mainfrom
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
Skipping CI for Draft Pull Request. |
|
Companion preset definition PR: openshift/release#75830 |
WalkthroughThis change introduces a registry-pull preset system for ProwJob generation. A new constant and builder methods enable selective application of a Prow preset that handles pull-secret credentials, removing the need for inline secret volumes when the preset is active. The preset is conditionally enabled based on specific author or commenter identities. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Commenter
participant Server as Prow Plugin Server
participant Builder as ProwJob Builder
participant PodSpec as PodSpec Modifier
participant ProwJob as Generated ProwJob
User->>Server: Trigger ProwJob generation
Server->>Builder: Create JobBaseBuilder
alt Commenter is "petr-muller"
Server->>Builder: UseRegistryPullPreset()
Builder->>Builder: Set useRegistryPullPreset flag
end
Server->>Builder: Build()
alt shouldUseRegistryPullPreset() is true
Builder->>Builder: Add registry-pull label
Builder->>PodSpec: removeInlinePullSecret()
PodSpec->>PodSpec: Remove pull-secret volume/mount
end
Builder->>ProwJob: Return configured ProwJob
sequenceDiagram
participant PR as Pull Request
participant Reconciler as PR Queue Reconciler
participant PRAuthorCheck as Author Check
participant Builder as ProwJob Builder
participant PodSpec as PodSpec Modifier
participant ProwJob as Generated ProwJob
PR->>Reconciler: Trigger ProwJob generation
Reconciler->>PRAuthorCheck: hasPRAuthor(prs, "petr-muller")
PRAuthorCheck-->>Reconciler: Author matches?
Reconciler->>Builder: Create JobBaseBuilder
alt Author is "petr-muller"
Reconciler->>Builder: UseRegistryPullPreset()
end
Reconciler->>Builder: Build()
alt Registry pull preset enabled
Builder->>Builder: Add registry-pull label
Builder->>PodSpec: removeInlinePullSecret()
PodSpec->>PodSpec: Remove pull-secret volume/mount
end
Builder->>ProwJob: Return configured ProwJob
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Regenerate all Prowjob definitions using the modified prowgen that uses the preset-ci-operator-image-pull Prow preset instead of inlining the pull-secret volume and volumeMount in every job PodSpec. Per-job changes: - Added label: preset-ci-operator-image-pull: "true" - Removed: pull-secret volumeMount (3 lines) - Removed: pull-secret volume definition (3 lines) - Net: -5 lines per job 25,666 files changed, -633K lines net reduction. Depends on: - Preset definition: openshift#75830 - Prowgen change: openshift/ci-tools#4994 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
62ec57c to
5db89a5
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/prowgen/podspec.go (1)
54-68:⚠️ Potential issue | 🟡 MinorEnsure the companion Prow preset is deployed before rolling out this change.
This change removes the inline "pull-secret" volume from all generated ProwJob podspecs and delegates it to the
preset-ci-operator-image-pullProw preset. The preset is now applied via a label on all generated jobs, but if the preset is not yet deployed in the Prow environment, jobs will fail with missing mount errors. Verify that the companion preset definition in openshift/release is deployed and active before this change rolls out.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/prowgen/podspec.go` around lines 54 - 68, The generated PodSpec no longer includes the inline "pull-secret" volume and now relies on the "preset-ci-operator-image-pull" preset, which can break jobs if the preset isn't deployed; to be safe, restore the inline pull-secret volume in pkg/prowgen/podspec.go by adding a Volume entry named "pull-secret" with a SecretVolumeSource pointing to the pull-secret Secret (the same name used by your mounts) alongside the existing Volumes (e.g., near the "result-aggregator" and "manifest-tool-local-pusher" entries) so jobs keep working until the companion preset is confirmed deployed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@pkg/prowgen/podspec.go`:
- Around line 54-68: The generated PodSpec no longer includes the inline
"pull-secret" volume and now relies on the "preset-ci-operator-image-pull"
preset, which can break jobs if the preset isn't deployed; to be safe, restore
the inline pull-secret volume in pkg/prowgen/podspec.go by adding a Volume entry
named "pull-secret" with a SecretVolumeSource pointing to the pull-secret Secret
(the same name used by your mounts) alongside the existing Volumes (e.g., near
the "result-aggregator" and "manifest-tool-local-pusher" entries) so jobs keep
working until the companion preset is confirmed deployed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a056f896-8c6e-457c-b1ee-94a2e86b6534
📒 Files selected for processing (137)
cmd/ci-operator-prowgen/testdata/zz_fixture_postsubmit_TestFromCIOperatorConfigToProwYaml_Custom_test_timeout.yamlcmd/ci-operator-prowgen/testdata/zz_fixture_postsubmit_TestFromCIOperatorConfigToProwYaml_Input_is_YAML_and_it_is_correctly_processed.yamlcmd/ci-operator-prowgen/testdata/zz_fixture_postsubmit_TestFromCIOperatorConfigToProwYaml_Using_a_variant_config__one_test_and_images__one_existing_job._Expect_one_presubmit__pre_post_submit_images_jobs._Existing_job_should_not_be_changed..yamlcmd/ci-operator-prowgen/testdata/zz_fixture_postsubmit_TestFromCIOperatorConfigToProwYaml_one_test_and_images__no_previous_jobs._Expect_test_presubmit__pre_post_submit_images_jobs.yamlcmd/ci-operator-prowgen/testdata/zz_fixture_presubmit_TestFromCIOperatorConfigToProwYaml_Custom_test_timeout.yamlcmd/ci-operator-prowgen/testdata/zz_fixture_presubmit_TestFromCIOperatorConfigToProwYaml_Input_is_YAML_and_it_is_correctly_processed.yamlcmd/ci-operator-prowgen/testdata/zz_fixture_presubmit_TestFromCIOperatorConfigToProwYaml_Using_a_variant_config__one_test_and_images__one_existing_job._Expect_one_presubmit__pre_post_submit_images_jobs._Existing_job_should_not_be_changed..yamlcmd/ci-operator-prowgen/testdata/zz_fixture_presubmit_TestFromCIOperatorConfigToProwYaml_one_test_and_images__no_previous_jobs._Expect_test_presubmit__pre_post_submit_images_jobs.yamlpkg/api/constant.gopkg/prowgen/jobbase.gopkg/prowgen/podspec.gopkg/prowgen/testdata/zz_fixture_TestCIPullSecret_secret_is_added.yamlpkg/prowgen/testdata/zz_fixture_TestClaims_secret_is_added.yamlpkg/prowgen/testdata/zz_fixture_TestCustomHashInput_custom_hash_input_is_added.yamlpkg/prowgen/testdata/zz_fixture_TestCustomHashInput_custom_hash_inputs_are_added.yamlpkg/prowgen/testdata/zz_fixture_TestGSMConfig_add_gsm_config_volume_and_mount.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobBase_config_variant.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobBase_expose_job_for_private_repos_with_public_results.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobBase_expose_option_set_but_not_private.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobBase_hidden_job_for_private_repos.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobBase_no_special_options.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobBase_path_alias.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobBase_rehearsable.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_Promotion_configuration_causes_promote_job.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_Promotion_configuration_causes_promote_job_with_unique_targets.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_cluster_label_for_periodic.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_cluster_label_for_postsubmit.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_cluster_label_for_presubmit.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_disabled_rehearsals_at_job_level.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_disabled_rehearsals_at_repo_level.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_images_job_is_configured_for_slack_reporting.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_kvm_label.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_multiarch_postsubmit_images.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_multiarch_test_job.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_no_Promotion_configuration_has_no_branch_job.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_operator_section_creates_bundle_with_capabilities.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_operator_section_creates_ci_index_my_bundle_presubmit_job.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_operator_section_creates_ci_index_presubmit_job.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_operator_section_without_index_creates_ci_index_my_bundle_presubmit_job.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_periodic_presubmit_with_capabilities.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_periodic_with_capabilities.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_periodic_with_presubmit.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_promotion_postsubmit_and_periodic_.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_sharded_presubmit.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_template_test.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_two_tests_and_empty_Images_so_only_two_test_presubmits_are_generated.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_two_tests_and_empty_Images_with_one_test_configured_as_a_postsubmit.yamlpkg/prowgen/testdata/zz_fixture_TestGenerateJobs_two_tests_and_nonempty_Images_so_two_test_presubmits_and_images_pre_postsubmits_are_generated_.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePeriodicForTest_periodic_for_a_test_in_a_variant_config.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePeriodicForTest_periodic_for_a_test_with_retry.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePeriodicForTest_periodic_for_standard_test.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePeriodicForTest_periodic_using_interval.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePeriodicForTest_periodic_using_minimum_interval.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePeriodicForTest_periodic_with_capabilities.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePeriodicForTest_periodic_with_disabled_rehearsal.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePostSubmitForTest_Lowercase_org_repo_and_branch.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePostSubmitForTest_Uppercase_org__repo_and_branch.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePostSubmitForTest_postsubmit_with_capabilities.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePostSubmitForTest_postsubmit_with_run_if_changed.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePostSubmitForTest_postsubmit_with_skip_if_only_changed.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_capabilities_added.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_optional_presubmit.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_presubmit_for_a_test_in_a_variant_config.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_presubmit_for_standard_test.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_presubmit_with_always_run_but_optional_true.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_presubmit_with_always_run_but_pipeline_run_if_changed_set.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_presubmit_with_always_run_but_pipeline_skip_if_only_changed_set.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_presubmit_with_always_run_but_run_if_changed_set.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_presubmit_with_always_run_false.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_presubmit_with_always_run_false_and_pipeline_run_if_changed.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_presubmit_with_always_run_false_and_pipeline_skip_if_only_changed.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_presubmit_with_run_if_changed.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_presubmit_with_skip_if_only_changed.yamlpkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_rehearsal_disabled.yamlpkg/prowgen/testdata/zz_fixture_TestGitHubToken_podspec_for_private_repo__reusing_Prow_s_volume_with_credentials.yamlpkg/prowgen/testdata/zz_fixture_TestGitHubToken_podspec_for_private_repo_without_reusing_Prow_s_volume_with_credentials.yamlpkg/prowgen/testdata/zz_fixture_TestInjectTestFrom_inject_coordinates_with_variant.yamlpkg/prowgen/testdata/zz_fixture_TestInjectTestFrom_inject_coordinates_without_variant.yamlpkg/prowgen/testdata/zz_fixture_TestLeaseClient_secret_is_added.yamlpkg/prowgen/testdata/zz_fixture_TestMiscellaneous_Cluster.yamlpkg/prowgen/testdata/zz_fixture_TestMiscellaneous_PathAlias.yamlpkg/prowgen/testdata/zz_fixture_TestMiscellaneous_Rehearsable.yamlpkg/prowgen/testdata/zz_fixture_TestMiscellaneous_TestName.yamlpkg/prowgen/testdata/zz_fixture_TestMiscellaneous_WithLabel.yamlpkg/prowgen/testdata/zz_fixture_TestNewCiOperatorPodSpecGenerator_defaults_repo.yamlpkg/prowgen/testdata/zz_fixture_TestNewCiOperatorPodSpecGenerator_no_parameter_is_added_when_variant_is_empty.yamlpkg/prowgen/testdata/zz_fixture_TestNewCiOperatorPodSpecGenerator_parameter_is_added_for_variant.yamlpkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_OpenshiftAnsibleClusterTestConfiguration.yamlpkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_OpenshiftAnsibleCustomClusterTestConfiguration.yamlpkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_OpenshiftInstallerClusterTestConfiguration.yamlpkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_OpenshiftInstallerCustomTestImageClusterTestConfiguration.yamlpkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_OpenshiftInstallerUPIClusterTestConfiguration.yamlpkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_job_excluded_by_patterns_should_not_have_slack_reporter_config.yamlpkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_literal_multi_stage_test.yamlpkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_multi_stage_test.yamlpkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_multi_stage_test_with_CSI_enabled.yamlpkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_multi_stage_test_with_claim.yamlpkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_multi_stage_test_with_cluster_profile.yamlpkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_multi_stage_test_with_releases.yamlpkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_simple_container_based_test.yamlpkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_simple_container_based_test_with_cluster.yamlpkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_simple_container_based_test_with_secret.yamlpkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_simple_container_based_test_with_secrets.yamlpkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_simple_container_based_test_with_timeout.yamlpkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_simple_container_based_test_with_timeout_and_no_decoration.yamlpkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_simple_test_with_CSI_enabled.yamlpkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_simple_with_slack_reporter_config.yamlpkg/prowgen/testdata/zz_fixture_TestPromotion_secret_and_parameters_are_added.yamlpkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_default_job_without_further_configuration.yamlpkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_default_job_without_further_configuration__including_podspec.yamlpkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_a_buildroot_in_of_openshift_release_main__does_not_have_no_builds__label.yamlpkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_a_variant.yamlpkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_a_variant__including_podspec.yamlpkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_binary_build_in_openshift_release_main__does_not_have_no_builds__label.yamlpkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_configured_prefix.yamlpkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_image_builds_in_of_openshift_release_main__does_not_have_no_builds__label.yamlpkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_latest_release_that_is_a_candidate__has_job_release__label.yamlpkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_latest_release_that_is_not_a_candidate__does_not_have_job_release__label.yamlpkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_no_builds_in_openshift_release_main__does_have_no_builds__label.yamlpkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_no_builds_outside_of_openshift_release_main__does_not_have_no_builds__label.yamlpkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_not_a_latest_release_that_is_a_candidate__does_not_have_job_release__label.yamlpkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_test_binary_build_in_of_openshift_release_main__does_not_have_no_builds__label.yamlpkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_private_job_with_cloning__including_podspec.yamlpkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_private_job_without_cloning__including_podspec.yamlpkg/prowgen/testdata/zz_fixture_TestReleaseInitial_add_release_initial.yamlpkg/prowgen/testdata/zz_fixture_TestReleaseLatest_add_release_latest.yamlpkg/prowgen/testdata/zz_fixture_TestReleaseRpms_envvar_additional_envvar_generated_for_template.yamlpkg/prowgen/testdata/zz_fixture_TestSecrets_empty_list_is_a_nop.yamlpkg/prowgen/testdata/zz_fixture_TestSecrets_multiple_secrets.yamlpkg/prowgen/testdata/zz_fixture_TestSecrets_one_secret.yamlpkg/prowgen/testdata/zz_fixture_TestTargetAdditionalSuffix_target_additional_suffix_is_added.yamlpkg/prowgen/testdata/zz_fixture_TestTargets_multiple_targets.yamlpkg/prowgen/testdata/zz_fixture_TestTargets_single_target.yamlpkg/prowgen/testdata/zz_fixture_TestTemplate_different_template_with_command.yamlpkg/prowgen/testdata/zz_fixture_TestTemplate_template_with_a_custom_test_image.yamlpkg/prowgen/testdata/zz_fixture_TestTemplate_template_with_command.yamlpkg/prowgen/testdata/zz_fixture_TestTemplate_template_with_different_command.yaml
💤 Files with no reviewable changes (27)
- pkg/prowgen/testdata/zz_fixture_TestPromotion_secret_and_parameters_are_added.yaml
- pkg/prowgen/testdata/zz_fixture_TestNewCiOperatorPodSpecGenerator_parameter_is_added_for_variant.yaml
- pkg/prowgen/testdata/zz_fixture_TestReleaseInitial_add_release_initial.yaml
- pkg/prowgen/testdata/zz_fixture_TestClaims_secret_is_added.yaml
- pkg/prowgen/testdata/zz_fixture_TestTemplate_different_template_with_command.yaml
- pkg/prowgen/testdata/zz_fixture_TestInjectTestFrom_inject_coordinates_without_variant.yaml
- pkg/prowgen/testdata/zz_fixture_TestNewCiOperatorPodSpecGenerator_no_parameter_is_added_when_variant_is_empty.yaml
- pkg/prowgen/testdata/zz_fixture_TestTargets_single_target.yaml
- pkg/prowgen/testdata/zz_fixture_TestCIPullSecret_secret_is_added.yaml
- pkg/prowgen/testdata/zz_fixture_TestReleaseRpms_envvar_additional_envvar_generated_for_template.yaml
- pkg/prowgen/testdata/zz_fixture_TestSecrets_one_secret.yaml
- pkg/prowgen/testdata/zz_fixture_TestGitHubToken_podspec_for_private_repo_without_reusing_Prow_s_volume_with_credentials.yaml
- pkg/prowgen/testdata/zz_fixture_TestSecrets_multiple_secrets.yaml
- pkg/prowgen/testdata/zz_fixture_TestTemplate_template_with_a_custom_test_image.yaml
- pkg/prowgen/testdata/zz_fixture_TestGitHubToken_podspec_for_private_repo__reusing_Prow_s_volume_with_credentials.yaml
- pkg/prowgen/testdata/zz_fixture_TestInjectTestFrom_inject_coordinates_with_variant.yaml
- pkg/prowgen/testdata/zz_fixture_TestTargetAdditionalSuffix_target_additional_suffix_is_added.yaml
- pkg/prowgen/testdata/zz_fixture_TestGSMConfig_add_gsm_config_volume_and_mount.yaml
- pkg/prowgen/testdata/zz_fixture_TestCustomHashInput_custom_hash_inputs_are_added.yaml
- pkg/prowgen/testdata/zz_fixture_TestSecrets_empty_list_is_a_nop.yaml
- pkg/prowgen/testdata/zz_fixture_TestLeaseClient_secret_is_added.yaml
- pkg/prowgen/testdata/zz_fixture_TestReleaseLatest_add_release_latest.yaml
- pkg/prowgen/testdata/zz_fixture_TestNewCiOperatorPodSpecGenerator_defaults_repo.yaml
- pkg/prowgen/testdata/zz_fixture_TestTemplate_template_with_different_command.yaml
- pkg/prowgen/testdata/zz_fixture_TestTemplate_template_with_command.yaml
- pkg/prowgen/testdata/zz_fixture_TestCustomHashInput_custom_hash_input_is_added.yaml
- pkg/prowgen/testdata/zz_fixture_TestTargets_multiple_targets.yaml
5db89a5 to
17c9c43
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates prowgen to rely on a Prow preset for mounting registry pull credentials, rather than hardcoding the pull-secret volume/volumeMount into the generated PodSpec, and adds a label to all prowgen-generated jobs so the preset can be applied.
Changes:
- Remove the
pull-secretvolume and volumeMount fromdefaultPodSpecso it can be provided via a Prow preset. - Add a new job label (
preset-ci-operator-image-pull: "true") to prowgen-generated jobs to trigger the preset. - Update test fixtures to reflect the removed volume/mount and the newly added label.
Reviewed changes
Copilot reviewed 137 out of 137 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/prowgen/podspec.go | Removes hardcoded pull-secret volume/mount from the default PodSpec (now expected via preset). |
| pkg/prowgen/jobbase.go | Adds the preset-trigger label to the base labels for generated jobs. |
| pkg/api/constant.go | Introduces a constant for the new preset-trigger label key. |
| pkg/prowgen/testdata/zz_fixture_TestTemplate_template_with_different_command.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestTemplate_template_with_command.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestTemplate_template_with_a_custom_test_image.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestTemplate_different_template_with_command.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestTargets_single_target.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestTargets_multiple_targets.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestTargetAdditionalSuffix_target_additional_suffix_is_added.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestSecrets_one_secret.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestSecrets_multiple_secrets.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestSecrets_empty_list_is_a_nop.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestReleaseRpms_envvar_additional_envvar_generated_for_template.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestReleaseLatest_add_release_latest.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestReleaseInitial_add_release_initial.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_private_job_without_cloning__including_podspec.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_private_job_with_cloning__including_podspec.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_test_binary_build_in_of_openshift_release_main__does_not_have_no_builds__label.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_not_a_latest_release_that_is_a_candidate__does_not_have_job_release__label.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_no_builds_outside_of_openshift_release_main__does_not_have_no_builds__label.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_no_builds_in_openshift_release_main__does_have_no_builds__label.yaml | Fixture update: add preset label alongside existing labels. |
| pkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_latest_release_that_is_not_a_candidate__does_not_have_job_release__label.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_latest_release_that_is_a_candidate__has_job_release__label.yaml | Fixture update: add preset label alongside existing labels. |
| pkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_image_builds_in_of_openshift_release_main__does_not_have_no_builds__label.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_configured_prefix.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_binary_build_in_openshift_release_main__does_not_have_no_builds__label.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_a_variant__including_podspec.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_a_variant.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_job_with_a_buildroot_in_of_openshift_release_main__does_not_have_no_builds__label.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_default_job_without_further_configuration__including_podspec.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestProwJobBaseBuilder_default_job_without_further_configuration.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestPromotion_secret_and_parameters_are_added.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_simple_with_slack_reporter_config.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_simple_test_with_CSI_enabled.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_simple_container_based_test_with_timeout_and_no_decoration.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_simple_container_based_test_with_timeout.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_simple_container_based_test_with_secrets.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_simple_container_based_test_with_secret.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_simple_container_based_test_with_cluster.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_simple_container_based_test.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_multi_stage_test_with_releases.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_multi_stage_test_with_cluster_profile.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_multi_stage_test_with_claim.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_multi_stage_test_with_CSI_enabled.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_multi_stage_test.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_literal_multi_stage_test.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_job_excluded_by_patterns_should_not_have_slack_reporter_config.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_OpenshiftInstallerUPIClusterTestConfiguration.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_OpenshiftInstallerCustomTestImageClusterTestConfiguration.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_OpenshiftInstallerClusterTestConfiguration.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_OpenshiftAnsibleCustomClusterTestConfiguration.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestNewProwJobBaseBuilderForTest_OpenshiftAnsibleClusterTestConfiguration.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestNewCiOperatorPodSpecGenerator_parameter_is_added_for_variant.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestNewCiOperatorPodSpecGenerator_no_parameter_is_added_when_variant_is_empty.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestNewCiOperatorPodSpecGenerator_defaults_repo.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestMiscellaneous_WithLabel.yaml | Fixture update: add preset label alongside existing labels. |
| pkg/prowgen/testdata/zz_fixture_TestMiscellaneous_TestName.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestMiscellaneous_Rehearsable.yaml | Fixture update: add preset label alongside existing labels. |
| pkg/prowgen/testdata/zz_fixture_TestMiscellaneous_PathAlias.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestMiscellaneous_Cluster.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestLeaseClient_secret_is_added.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestInjectTestFrom_inject_coordinates_without_variant.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestInjectTestFrom_inject_coordinates_with_variant.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestGitHubToken_podspec_for_private_repo_without_reusing_Prow_s_volume_with_credentials.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestGitHubToken_podspec_for_private_repo__reusing_Prow_s_volume_with_credentials.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_rehearsal_disabled.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_presubmit_with_skip_if_only_changed.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_presubmit_with_run_if_changed.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_presubmit_with_always_run_false_and_pipeline_skip_if_only_changed.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_presubmit_with_always_run_false_and_pipeline_run_if_changed.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_presubmit_with_always_run_false.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_presubmit_with_always_run_but_run_if_changed_set.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_presubmit_with_always_run_but_pipeline_skip_if_only_changed_set.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_presubmit_with_always_run_but_pipeline_run_if_changed_set.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_presubmit_with_always_run_but_optional_true.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_presubmit_for_standard_test.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_presubmit_for_a_test_in_a_variant_config.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_optional_presubmit.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePresubmitForTest_capabilities_added.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePostSubmitForTest_postsubmit_with_skip_if_only_changed.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePostSubmitForTest_postsubmit_with_run_if_changed.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePostSubmitForTest_postsubmit_with_capabilities.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePostSubmitForTest_Uppercase_org__repo_and_branch.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePostSubmitForTest_Lowercase_org_repo_and_branch.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePeriodicForTest_periodic_with_disabled_rehearsal.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePeriodicForTest_periodic_with_capabilities.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePeriodicForTest_periodic_using_minimum_interval.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePeriodicForTest_periodic_using_interval.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePeriodicForTest_periodic_for_standard_test.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePeriodicForTest_periodic_for_a_test_with_retry.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGeneratePeriodicForTest_periodic_for_a_test_in_a_variant_config.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_two_tests_and_nonempty_Images_so_two_test_presubmits_and_images_pre_postsubmits_are_generated_.yaml | Fixture update: add preset label(s). |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_two_tests_and_empty_Images_with_one_test_configured_as_a_postsubmit.yaml | Fixture update: add preset label(s). |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_two_tests_and_empty_Images_so_only_two_test_presubmits_are_generated.yaml | Fixture update: add preset label(s). |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_template_test.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_sharded_presubmit.yaml | Fixture update: add preset label(s). |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_promotion_postsubmit_and_periodic_.yaml | Fixture update: add preset label(s); remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_periodic_with_presubmit.yaml | Fixture update: add preset label(s); remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_periodic_with_capabilities.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_periodic_presubmit_with_capabilities.yaml | Fixture update: add preset label(s); remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_operator_section_without_index_creates_ci_index_my_bundle_presubmit_job.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_operator_section_creates_ci_index_presubmit_job.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_operator_section_creates_ci_index_my_bundle_presubmit_job.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_operator_section_creates_bundle_with_capabilities.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_no_Promotion_configuration_has_no_branch_job.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_multiarch_test_job.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_multiarch_postsubmit_images.yaml | Fixture update: add preset label(s). |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_kvm_label.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_images_job_is_configured_for_slack_reporting.yaml | Fixture update: add preset label(s). |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_disabled_rehearsals_at_repo_level.yaml | Fixture update: add preset label(s); remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_disabled_rehearsals_at_job_level.yaml | Fixture update: add preset label(s); remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_cluster_label_for_presubmit.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_cluster_label_for_postsubmit.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_cluster_label_for_periodic.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_Promotion_configuration_causes_promote_job_with_unique_targets.yaml | Fixture update: add preset label(s); remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobs_Promotion_configuration_causes_promote_job.yaml | Fixture update: add preset label(s). |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobBase_rehearsable.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobBase_path_alias.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobBase_no_special_options.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobBase_hidden_job_for_private_repos.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobBase_expose_option_set_but_not_private.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobBase_expose_job_for_private_repos_with_public_results.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGenerateJobBase_config_variant.yaml | Fixture update: add preset label. |
| pkg/prowgen/testdata/zz_fixture_TestGSMConfig_add_gsm_config_volume_and_mount.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestCustomHashInput_custom_hash_inputs_are_added.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestCustomHashInput_custom_hash_input_is_added.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestClaims_secret_is_added.yaml | Fixture update: remove pull-secret mount/volume. |
| pkg/prowgen/testdata/zz_fixture_TestCIPullSecret_secret_is_added.yaml | Fixture update: remove pull-secret mount/volume. |
| cmd/ci-operator-prowgen/testdata/zz_fixture_presubmit_TestFromCIOperatorConfigToProwYaml_one_test_and_images__no_previous_jobs._Expect_test_presubmit__pre_post_submit_images_jobs.yaml | Fixture update: add preset label(s); remove pull-secret mount/volume. |
| cmd/ci-operator-prowgen/testdata/zz_fixture_presubmit_TestFromCIOperatorConfigToProwYaml_Using_a_variant_config__one_test_and_images__one_existing_job._Expect_one_presubmit__pre_post_submit_images_jobs._Existing_job_should_not_be_changed..yaml | Fixture update: add preset label(s); remove pull-secret mount/volume. |
| cmd/ci-operator-prowgen/testdata/zz_fixture_presubmit_TestFromCIOperatorConfigToProwYaml_Input_is_YAML_and_it_is_correctly_processed.yaml | Fixture update: add preset label(s); remove pull-secret mount/volume. |
| cmd/ci-operator-prowgen/testdata/zz_fixture_presubmit_TestFromCIOperatorConfigToProwYaml_Custom_test_timeout.yaml | Fixture update: add preset label(s); remove pull-secret mount/volume. |
| cmd/ci-operator-prowgen/testdata/zz_fixture_postsubmit_TestFromCIOperatorConfigToProwYaml_one_test_and_images__no_previous_jobs._Expect_test_presubmit__pre_post_submit_images_jobs.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| cmd/ci-operator-prowgen/testdata/zz_fixture_postsubmit_TestFromCIOperatorConfigToProwYaml_Using_a_variant_config__one_test_and_images__one_existing_job._Expect_one_presubmit__pre_post_submit_images_jobs._Existing_job_should_not_be_changed..yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| cmd/ci-operator-prowgen/testdata/zz_fixture_postsubmit_TestFromCIOperatorConfigToProwYaml_Input_is_YAML_and_it_is_correctly_processed.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
| cmd/ci-operator-prowgen/testdata/zz_fixture_postsubmit_TestFromCIOperatorConfigToProwYaml_Custom_test_timeout.yaml | Fixture update: add preset label; remove pull-secret mount/volume. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| VolumeMounts: []corev1.VolumeMount{ | ||
| { | ||
| Name: "pull-secret", | ||
| MountPath: "/etc/pull-secret", | ||
| ReadOnly: true, | ||
| }, | ||
| // pull-secret mount provided by preset-ci-operator-image-pull preset | ||
| { |
There was a problem hiding this comment.
pkg/prowgen/podspec.go currently imports github.com/openshift/ci-tools/pkg/api twice (once as api and once as cioperatorapi). Go rejects duplicate import paths, so this file will not compile as-is. Please keep a single import (choose one name) and update references accordingly.
17c9c43 to
8209018
Compare
Move the pull-secret volume and volumeMount from the hardcoded defaultPodSpec to a Prow preset. This is a proof of concept for using compositional Prow presets to deduplicate the ~28 lines of boilerplate volumes/mounts that are inlined into every one of the 126K+ generated Prowjob definitions. The preset is defined in openshift/release and matched via a new label added to all prowgen-generated jobs. The --image-import-pull-secret arg remains inline since presets cannot inject container args. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
8209018 to
6d44ca2
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 156 out of 156 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - mountPath: /secrets/manifest-tool | ||
| name: manifest-tool-local-pusher | ||
| readOnly: true | ||
| - mountPath: /etc/pull-secret | ||
| name: pull-secret | ||
| readOnly: true | ||
| - mountPath: /etc/report | ||
| name: result-aggregator | ||
| readOnly: true |
There was a problem hiding this comment.
This ProwJob fixture still runs ci-operator with --image-import-pull-secret=/etc/pull-secret/.dockerconfigjson, but the /etc/pull-secret volumeMount was removed and the ProwJob’s metadata.labels don’t include presets.ci.openshift.io/registry-pull: "true". Without the label, the registry-pull preset won’t be applied and ci-operator is likely to fail because the pull secret file won’t exist.
| volumeMounts: | ||
| - mountPath: /secrets/gcs | ||
| name: gcs-credentials | ||
| readOnly: true | ||
| - mountPath: /secrets/manifest-tool | ||
| name: manifest-tool-local-pusher | ||
| readOnly: true | ||
| - mountPath: /etc/pull-secret | ||
| name: pull-secret | ||
| readOnly: true | ||
| - mountPath: /etc/report | ||
| name: result-aggregator | ||
| readOnly: true |
There was a problem hiding this comment.
This ProwJob fixture references /etc/pull-secret/.dockerconfigjson via --image-import-pull-secret, but the /etc/pull-secret mount and volume are absent and the ProwJob metadata.labels also don’t include presets.ci.openshift.io/registry-pull: "true". The job will not receive the registry pull credentials unless that label is present (or the mount is restored).
| Args: []string{ | ||
| // /etc/pull-secret mount provided by registry-pull preset in Prow config | ||
| "--image-import-pull-secret=/etc/pull-secret/.dockerconfigjson", | ||
| "--gcs-upload-secret=/secrets/gcs/service-account.json", |
There was a problem hiding this comment.
defaultPodSpec now always passes --image-import-pull-secret=/etc/pull-secret/.dockerconfigjson, but the /etc/pull-secret volume+mount are no longer present in the spec. This will break any ProwJobs that don’t get the new registry-pull preset applied (e.g., code paths that create ProwJobs via pjutil.NewProwJob but don’t propagate JobBase.Labels). Ensure every generated ProwJob has the presets.ci.openshift.io/registry-pull: "true" label (or otherwise guarantee the mount exists) so the referenced file is present at runtime.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: petr-muller, Prucek The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold oh this really needs to be tested a bit before we break all jobs |
|
New changes are detected. LGTM label has been removed. |
Instead of applying the registry-pull preset globally, limit it to a small scope for production validation: - prowgen: only for openshift/cluster-version-operator on release-4.23 - prpqr: only when a PR author is petr-muller - multi-pr: only when the /testwith commenter is petr-muller The builder gains a UseRegistryPullPreset() method and a metadata-based check in Build(). When triggered, the preset label is added and the inline pull-secret volume/mount is removed via a PodSpec mutator. All other jobs retain the inline pull-secret volume unchanged. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
8a379ed to
c422ded
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // shouldUseRegistryPullPreset returns true if the job being built should use | ||
| // the registry-pull preset. Currently enabled for openshift/cluster-version-operator | ||
| // on the release-4.23 branch to validate the preset mechanism. | ||
| func (p *prowJobBaseBuilder) shouldUseRegistryPullPreset() bool { | ||
| if p.useRegistryPullPreset { | ||
| return true | ||
| } | ||
| return p.info != nil && | ||
| p.info.Org == "openshift" && | ||
| p.info.Repo == "cluster-version-operator" && | ||
| p.info.Branch == "release-4.23" | ||
| } |
There was a problem hiding this comment.
The defaulting logic in shouldUseRegistryPullPreset hard-codes a specific org/repo/branch to flip job behavior. This makes registry-pull preset usage implicit and hard to reason about, and it also conflicts with the updated ephemeralcluster ProwJob fixtures (which expect the preset label for other repos/branches). Prefer making this strictly opt-in via explicit builder configuration (or a config/feature-flag), and avoid embedding repo/branch policy in the generator.
| jobBaseGen := prowgen.NewProwJobBaseBuilderForTest(ciopConfig, fakeProwgenInfo, prowgen.NewCiOperatorPodSpecGenerator(), test) | ||
| if hasPRAuthor(prs, "petr-muller") { | ||
| jobBaseGen.UseRegistryPullPreset() | ||
| } |
There was a problem hiding this comment.
This conditional enables behavior based on a specific GitHub username. Shipping user-specific feature gates in controller logic is brittle and leads to inconsistent job specs depending on who authored a PR. Please replace this with a real feature flag/config switch (or remove it once validation is complete).
| fakeProwgenInfo := &prowgen.ProwgenInfo{Metadata: testJobMetadata} | ||
| jobBaseGen := prowgen.NewProwJobBaseBuilderForTest(ciopConfig, fakeProwgenInfo, prowgen.NewCiOperatorPodSpecGenerator(), test) | ||
| if commenter == "petr-muller" { | ||
| jobBaseGen.UseRegistryPullPreset() | ||
| } | ||
| jobBaseGen.PodSpec.Add(prowgen.InjectTestFrom(&jr.JobMetadata)) |
There was a problem hiding this comment.
Hard-coding a specific commenter username to alter generated ProwJobs will create surprising, user-dependent behavior and is difficult to maintain. Prefer a configuration/feature-flag based toggle (or remove after the experiment).
| fakeProwgenInfo := &prowgen.ProwgenInfo{Metadata: testJobMetadata} | ||
| jobBaseGen := prowgen.NewProwJobBaseBuilderForTest(ciopConfig, fakeProwgenInfo, prowgen.NewCiOperatorPodSpecGenerator(), test) | ||
| if commenter == "petr-muller" { | ||
| jobBaseGen.UseRegistryPullPreset() | ||
| } | ||
| jobBaseGen.PodSpec.Add(prowgen.InjectTestFrom(&jr.JobMetadata)) |
There was a problem hiding this comment.
The new registry-pull preset path (triggered by the commenter check) isn't covered by tests. Please add a TestGenerateProwJob case that passes a matching commenter and asserts the preset label is set and the inline pull-secret volume/mount is removed.
| created-by-prow: "true" | ||
| event-GUID: aa9ede40-8fbb-11f0-8717-9eed0c1315d0 | ||
| pj-rehearse.openshift.io/can-be-rehearsed: "true" | ||
| presets.ci.openshift.io/registry-pull: "true" | ||
| prow.k8s.io/context: cluster-provisioning |
There was a problem hiding this comment.
This fixture now expects the registry-pull preset label and removal of the inline pull-secret volume/mount. However, the current generator only enables that behavior for explicit UseRegistryPullPreset() calls (or a specific openshift/cluster-version-operator release-4.23 default). For this job's org/repo/branch, the label/mount removal will not occur, so the fixture will diverge from actual output and tests will fail. Either enable the preset for this flow in code or revert this fixture change.
| created-by-prow: "true" | ||
| event-GUID: aa9ede40-8fbb-11f0-8717-9eed0c1315d0 | ||
| pj-rehearse.openshift.io/can-be-rehearsed: "true" | ||
| presets.ci.openshift.io/registry-pull: "true" | ||
| prow.k8s.io/context: cluster-provisioning |
There was a problem hiding this comment.
This fixture now expects the registry-pull preset label and removal of the inline pull-secret volume/mount, but the generator doesn't enable the preset for openshift/ci-tools/main unless explicitly requested. This will make the fixture inconsistent with actual generated ProwJobs. Either update the generation code to set the preset label for this case or revert the fixture change.
| created-by-prow: "true" | ||
| event-GUID: aa9ede40-8fbb-11f0-8717-9eed0c1315d0 | ||
| pj-rehearse.openshift.io/can-be-rehearsed: "true" | ||
| presets.ci.openshift.io/registry-pull: "true" | ||
| prow.k8s.io/context: cluster-provisioning |
There was a problem hiding this comment.
This fixture now expects the registry-pull preset label and removal of the inline pull-secret volume/mount, but the generator doesn't enable the preset for openshift/ci-tools/main unless explicitly requested. This will make the fixture inconsistent with actual generated ProwJobs. Either update the generation code to set the preset label for this case or revert the fixture change.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
cmd/multi-pr-prow-plugin/server.go (1)
357-359: Hardcoded username for limited rollout validation is acceptable but should be documented.The hardcoded check for
"petr-muller"aligns with the PR's stated goal of narrow validation before broader rollout. Consider adding a brief inline comment explaining this is a temporary rollout gate, similar to the comment inshouldUseRegistryPullPreset().💡 Suggested comment for clarity
jobBaseGen := prowgen.NewProwJobBaseBuilderForTest(ciopConfig, fakeProwgenInfo, prowgen.NewCiOperatorPodSpecGenerator(), test) + // Temporarily gate registry-pull preset to specific user for validation if commenter == "petr-muller" { jobBaseGen.UseRegistryPullPreset() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/multi-pr-prow-plugin/server.go` around lines 357 - 359, Add a brief inline comment above the hardcoded check "if commenter == \"petr-muller\"" explaining that this username-based branch is an intentional, temporary rollout gate for limited validation before broader rollout; reference the same rationale as shouldUseRegistryPullPreset() and mark it as temporary so future maintainers know to remove/expand it later, leaving the condition and the call to jobBaseGen.UseRegistryPullPreset() unchanged.cmd/multi-pr-prow-plugin/server_test.go (1)
996-996: Test correctly updated to pass empty commenter, but consider adding coverage for the preset-enabled path.The empty string ensures existing tests continue to verify non-preset behavior. For comprehensive coverage of the new functionality, consider adding a test case where
commenter == "petr-muller"to verify the preset label is applied and inline pull-secret is removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/multi-pr-prow-plugin/server_test.go` at line 996, Add a new test case that exercises the preset-enabled path by calling s.generateProwJob(tc.jobRun, "petr-muller") (instead of the current empty commenter) and assert that the returned prowJob contains the expected preset label in prowJob.Labels and that any inline pull-secret was removed from the job spec (e.g., prowJob.Spec.PodSpec.ImagePullSecrets or the inline secret field you use). Use the existing generateProwJob and jobRun setup to create the input, then add assertions for the preset label key/value and that no inline pull-secret remains to validate the preset behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmd/multi-pr-prow-plugin/server_test.go`:
- Line 996: Add a new test case that exercises the preset-enabled path by
calling s.generateProwJob(tc.jobRun, "petr-muller") (instead of the current
empty commenter) and assert that the returned prowJob contains the expected
preset label in prowJob.Labels and that any inline pull-secret was removed from
the job spec (e.g., prowJob.Spec.PodSpec.ImagePullSecrets or the inline secret
field you use). Use the existing generateProwJob and jobRun setup to create the
input, then add assertions for the preset label key/value and that no inline
pull-secret remains to validate the preset behavior.
In `@cmd/multi-pr-prow-plugin/server.go`:
- Around line 357-359: Add a brief inline comment above the hardcoded check "if
commenter == \"petr-muller\"" explaining that this username-based branch is an
intentional, temporary rollout gate for limited validation before broader
rollout; reference the same rationale as shouldUseRegistryPullPreset() and mark
it as temporary so future maintainers know to remove/expand it later, leaving
the condition and the call to jobBaseGen.UseRegistryPullPreset() unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ed3e1ea6-edae-4cce-9b68-54c98f9a5389
📒 Files selected for processing (5)
cmd/multi-pr-prow-plugin/server.gocmd/multi-pr-prow-plugin/server_test.gopkg/controller/prpqr_reconciler/prpqr_reconciler.gopkg/prowgen/jobbase.gopkg/prowgen/podspec.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/prowgen/podspec.go
|
@petr-muller: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Summary
Proof of concept for using Prow presets to deduplicate boilerplate from generated Prowjob definitions.
This PR moves the
pull-secretvolume and volumeMount from the hardcodeddefaultPodSpecin prowgen to a Prow preset added in openshift/release#75830, matched via a new label added to all prowgen-generated jobs.Design
This is the first of several compositional, domain-specific presets — each covers one logical domain with its own label, and they compose independently:
Impact (this preset alone)
ci-operator/jobs/Companion PR
Requires the preset definition in openshift/release: openshift/release#75830
🤖 Generated with Claude Code
Summary by CodeRabbit