Skip to content

[BUILD-1606] feat: map S2I ForcePull to pull-policy parameter - #148

Open
psrvere wants to merge 2 commits into
migtools:mainfrom
psrvere:BUILD-1606-force-pull-s2i
Open

[BUILD-1606] feat: map S2I ForcePull to pull-policy parameter#148
psrvere wants to merge 2 commits into
migtools:mainfrom
psrvere:BUILD-1606-force-pull-s2i

Conversation

@psrvere

@psrvere psrvere commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Maps BuildConfig SourceStrategy.ForcePull=true to the Shipwright S2I strategy param pull-policy=always via new processSourceStrategyForcePull() (replaces the previous warn-only behavior)
  • Adds S2iForcePullParamName = "pull-policy" constant; ForcePull=false adds no param (strategy default if-not-present preserved)
  • Removes the now-obsolete ForcePullFlagS2iRFE constant from convert/rfe.go

Test plan

  • Unit tests: 87/87 PASS (GOWORK=off go test ./convert/...)
  • Cluster e2e (2026-07-28): converted Build with pull-policy=always applied on OpenShift; BuildRun test-conversion-run-1 Succeeded

Dependencies

Related

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Source-to-Image builds now honor the ForcePull setting by applying an always image pull policy during conversion.
  • Bug Fixes

    • Improved consistency between OpenShift Source strategies and generated Shipwright build settings.
  • Tests

    • Added coverage verifying pull-policy behavior when ForcePull is enabled or disabled.

psrvere and others added 2 commits July 27, 2026 09:50
…D-1606)

Replace the S2I ForcePull warning with actual parameter mapping. When a
BuildConfig has SourceStrategy.ForcePull=true, the converted Shipwright
Build now includes pull-policy="always" as a strategy parameter. Remove
ForcePullFlagS2iRFE from rfe.go as it is now implemented.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Signed-off-by: Prateek Singh Rathore <prateek.singh.rathore@gmail.com>
MockClient.Apply referenced runtime.ApplyConfiguration/client.ApplyOption,
which only exist under go.work resolution (controller-runtime v0.23.3 via
crane). crane-lib pins v0.21.0, so CI standalone builds failed. Also
resolves leftover conflict markers in buildconfigs.go param constants.
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 22e456cd-f9e9-498e-bf6d-a68a681c3e15

📥 Commits

Reviewing files that changed from the base of the PR and between 0e33157 and 53579d7.

📒 Files selected for processing (3)
  • convert/buildconfigs.go
  • convert/buildconfigs_test.go
  • convert/rfe.go
💤 Files with no reviewable changes (1)
  • convert/rfe.go

📝 Walkthrough

Walkthrough

Source strategy conversion now maps ForcePull to Shipwright’s pull-policy=always parameter. The change adds focused tests for enabled and disabled cases and removes an obsolete RFE constant.

Changes

Source ForcePull conversion

Layer / File(s) Summary
Source strategy parameter mapping
convert/buildconfigs.go, convert/rfe.go
Source strategy conversion appends pull-policy=always when ForcePull is enabled, using the new exported parameter constant; the obsolete RFE constant is removed.
ForcePull conversion tests
convert/buildconfigs_test.go
Tests verify that enabled ForcePull produces one pull-policy=always parameter and disabled ForcePull produces none.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SourceStrategy
  participant ConvertOptions
  participant ShipwrightBuild
  SourceStrategy->>ConvertOptions: provide ForcePull
  ConvertOptions->>ShipwrightBuild: append pull-policy=always
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: mapping S2I ForcePull to the Shipwright pull-policy parameter.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant