Skip to content

Conversation

@psrvere
Copy link

@psrvere psrvere commented Oct 27, 2025

This PR includes migration of BuildSoure Type in BuildConfig to Shipwright equivalent specs. Following is the filed level status:

Field Name Status Notes
BuildSourceType Ignored. It is not used by BuildConfig Code Logic
BinaryBuildSource
     Files
     Archive Not supported in Shipwright
Dockerfile (Inline Dockerfile) Build-1495
GitBuildSource
Images
     Single Image
     Multiple Images
     As (alias) field
     Paths field
     From field
     PullSecret field
ContextDir
SourceSecret
Secrets RFE - Under Discussion
ConfigMaps RFE - Under Discussion

This PR also unifies logic for "from" field processing for docker and source strategy and corrects the implementation.

Here are the detailed notes

  - Added source processing logic for git, binary, images and dockerfile types in BuildSource of BuildConfig
  - Unified logic for "from" field processing for docker and source strategy
  - Updated tests

Signed-off-by: Prateek Rathore <prathore@redhat.com>
Copy link
Contributor

@jmontleon jmontleon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK / LGTM

@jmontleon
Copy link
Contributor

jmontleon commented Nov 3, 2025

A couple of the test failures can be fixed pretty easily, I think:

diff --git a/convert/buildconfigs_test.go b/convert/buildconfigs_test.go
index 24475bc..e7818f3 100644
--- a/convert/buildconfigs_test.go
+++ b/convert/buildconfigs_test.go
@@ -236,7 +236,9 @@ func TestProcessSource(t *testing.T) {
 
        for _, tt := range tests {
                t.Run(tt.name, func(t *testing.T) {
-                       co := &ConvertOptions{}
+                       co := &ConvertOptions{
+                               logger: logrus.New(),
+                       }
                        build := &shipwrightv1beta1.Build{}
 
                        co.processSource(tt.buildConfig, build)
@@ -808,7 +810,7 @@ func TestProcessSourceStrategyFromField(t *testing.T) {
 
                err := co.processStrategyFromField(bc, build)
                assert.Error(t, err)
-               assert.Contains(t, err.Error(), "source strategy From kind Unknown is unknown for BuildConfig test-bc")
+               assert.Contains(t, err.Error(), "strategy 'From' kind Unknown is unknown type Source for BuildConfig test-bc")
                assert.Equal(t, 0, len(build.Spec.ParamValues))
        })

There is a third if I am not mistaken because a ParamValue is no longer being set and either should be adjusted or removed.

Signed-off-by: Prateek Rathore <prathore@redhat.com>
@psrvere
Copy link
Author

psrvere commented Nov 4, 2025

Thanks @jmontleon for the review. I have fixed the tests and these are passing now.

@jmontleon jmontleon merged commit 3e14852 into migtools:shipwright Nov 4, 2025
1 check passed
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.

2 participants