fix extra args#41
Merged
Merged
Conversation
and some extra Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>
📝 WalkthroughWalkthroughThis PR extends image build flexibility by introducing AIB extra arguments support, implementing artifact filename propagation through the build pipeline, adding tar.gz compression detection, removing strict format validation, and expanding artifact push media type handling. Changes
Sequence DiagramssequenceDiagram
actor User
participant CLI as CLI (caib)
participant Server as API Server
participant Controller as Build Controller
participant Pipeline as Tekton Pipeline
participant Container as Build Container
User->>CLI: caib build --aib-extra-args="..."
CLI->>Server: CreateBuildRequest(AIBExtraArgs)
Server->>Pipeline: Create PipelineRun
Pipeline->>Container: Execute build-image<br/>(with AIB_EXTRA_ARGS)
Container->>Pipeline: Return artifact-filename result
Pipeline->>Controller: PipelineRun completed
Controller->>Controller: extractArtifactFilename()
Controller->>Pipeline: Create push TaskRun<br/>(with artifactFilename)
sequenceDiagram
participant Controller as Build Controller
participant PipelineRun as PipelineRun Results
participant PushTask as Push TaskRun
participant Registry as Container Registry
Controller->>PipelineRun: Fetch artifact-filename result
PipelineRun-->>Controller: artifact-filename value
Controller->>Controller: Validate artifactFilename
Controller->>PushTask: createPushTaskRun(imageBuild, artifactFilename)
PushTask->>Registry: Push artifact using filename
Registry-->>PushTask: Success
PushTask-->>Controller: Task completed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/buildapi/server.go (1)
864-873: Broadened push secret condition is correct, but error message could be clearer.The logic correctly creates a push secret when either
PushRepositoryorExportOCIis specified. However, the error message on line 867 only mentions "push repository", which may confuse users when they specifyExportOCIwithout credentials.Suggested improvement for error message clarity
// Create push secret if pushing to registry (PushRepository for bootc, ExportOCI for disk images) if req.PushRepository != "" || req.ExportOCI != "" { if req.RegistryCredentials == nil || !req.RegistryCredentials.Enabled { - return "", "", fmt.Errorf("registry credentials are required when push repository is specified") + return "", "", fmt.Errorf("registry credentials are required when pushing to registry (push repository or export OCI)") }
🧹 Nitpick comments (1)
internal/controller/imagebuild/controller.go (1)
754-766: Consider adding early validation for empty artifact filename.The current flow relies on
createPushTaskRunto validate the empty filename, but adding an early check here would provide a more specific error message at the point of extraction failure.♻️ Optional: Add early validation
artifactFilename := extractArtifactFilename(pipelineRun) + if artifactFilename == "" { + log.Error(nil, "Failed to extract artifact filename from PipelineRun results") + msg := "Build completed but artifact filename not found in results" + if statusErr := r.updateStatus(ctx, imageBuild, phaseFailed, msg); statusErr != nil { + return ctrl.Result{}, statusErr + } + return ctrl.Result{}, nil + } // No push TaskRun yet, create one if err := r.createPushTaskRun(ctx, imageBuild, artifactFilename); err != nil {
Merged
This was referenced Feb 14, 2026
Merged
10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
and some extra
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.