remove tar based upload in favor of cat based

[bennyz]

This allows using ubi-minimal instead of nginx-unprivileged for the upload server.

Also:

• reuse cache pvc with workspaces

Summary by CodeRabbit

• Bug Fixes

  • Improved volume configuration logic for memory and PVC scratch volumes.
  • Enhanced storage class resolution with fallback to operator configuration.

• New Features

  • Optimized file transfer to pods using raw-byte streaming.
  • Enabled reuse of existing build-cache PVC for uploads when available.
  • Enhanced error reporting with captured stderr details.

• Tests

  • Added comprehensive unit tests for file copy functionality.

[coderabbitai]

Warning

Rate limit exceeded

@bennyz has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 1 minutes and 50 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 1 minutes and 50 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d0534266-c378-4118-9575-bcde5c80c3be

📥 Commits

Reviewing files that changed from the base of the PR and between 00c881e and b2ecb46.

📒 Files selected for processing (4)

• internal/buildapi/server.go
• internal/buildapi/server_upload_test.go
• internal/common/tasks/tasks.go
• internal/controller/imagebuild/controller.go

📝 Walkthrough

Walkthrough

This PR refactors the file-copy-to-pod mechanism from tar-based streaming to raw-byte streaming via shell commands, adds comprehensive unit tests for the updated logic, refines memory-backed volume configuration for scratch volumes, and improves PVC reuse and storage class resolution in the image build controller.

Changes

Cohort / File(s)	Summary
File Copy Mechanism internal/buildapi/server.go, internal/buildapi/server_upload_test.go	Replaced tar-based file transfer with direct raw-byte streaming into pods via stdin; added newPodExecExecutorFn for executor creation; updated error handling to capture stderr; introduced unit tests covering raw-byte streaming and error propagation.
Volume Configuration internal/common/tasks/tasks.go	Removed controller-runtime logger; adjusted GenerateBuildAutomotiveImageTask to apply memory-backed EmptyDir to container-storage unconditionally, and to scratch volumes only when PVC scratch volumes are disabled.
PVC and Upload Configuration internal/controller/imagebuild/controller.go	Updated createUploadPod to reuse BuildCachePVC when available; changed upload pod image from nginx to UBI10-minimal; extended storage class resolution to fall back to operator config when unset.

Sequence Diagram

sequenceDiagram
    actor Caller
    participant copyFileToPod
    participant newPodExecExecutorFn
    participant Pod
    participant Shell

    Caller->>copyFileToPod: copyFileToPod(fileContent)
    copyFileToPod->>newPodExecExecutorFn: Create executor
    newPodExecExecutorFn->>Pod: Initialize SPDY executor
    newPodExecExecutorFn-->>copyFileToPod: Return executor
    
    copyFileToPod->>Pod: StreamWithContext(cmd, stdin, stdout, stderr)
    Pod->>Shell: Execute: /bin/sh -c "mkdir -p ... && cat > file && chmod 0600"
    Shell->>Shell: Create directory
    Shell->>Shell: Write raw bytes from stdin to file
    Shell->>Shell: Apply chmod 0600
    Shell-->>Pod: Return exit code
    
    Pod-->>copyFileToPod: Return result
    alt Success
        copyFileToPod-->>Caller: Return nil
    else Failure
        copyFileToPod-->>Caller: Return error with stderr context
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• initial workspace #147: Modifies internal/buildapi/server.go workspace functionality alongside these file-copy changes.
• fix file upload #57: Updates PVC/workspace selection logic in internal/controller/imagebuild/controller.go similarly introducing PVC reuse patterns.
• optimizations and improvements to build process #185: Adjusts memory-backed EmptyDir volume logic in internal/common/tasks/tasks.go that this PR also modifies.

Suggested reviewers

• bkhizgiy

Poem

🐰 Raw bytes flow swift through stdin's stream,
No tar tar overhead in our build dream,
PVCs reused and volumes align,
Tests guard the path—a change so divine! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'remove tar based upload in favor of cat based' directly and clearly summarizes the main change across multiple files: replacing tar-based file transfer with raw-byte streaming (cat-based approach) in the copyFileToPod implementation.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/buildapi/server.go`:
- Around line 2598-2608: Add a request-scoped context to the upload flow and use
it for streaming to the pod: add a ctx context.Context field to the
uploadContext struct, set uploadCtx.ctx = c.Request.Context() in uploadFiles,
propagate that ctx parameter through processFilePart and into copyFileToPod
signatures/calls, and replace the hardcoded context.Background() passed to
executor.StreamWithContext(...) with the passed uploadContext.ctx so client
disconnects and server shutdown cancel the pod copy.

In `@internal/controller/imagebuild/controller.go`:
- Around line 1739-1748: The code reuses imageBuild.Spec.BuildCachePVC without
validating it; change the branch that sets workspacePVCName =
imageBuild.Spec.BuildCachePVC to perform a direct GET of that PVC (using the
controller's Kubernetes client) to ensure it exists and is accessible, and if
the GET returns not-found or an error, return that error so reconciliation fails
fast with a clear status; keep the existing fallback behavior that calls
r.getOrCreateWorkspacePVC when BuildCachePVC is empty, but replace the simple
assignment with a validation step that sets workspacePVCName only after
successful GET.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: aee99626-d090-4f96-b5c0-d7a454654e75

📥 Commits

Reviewing files that changed from the base of the PR and between 11afe49 and 00c881e.

📒 Files selected for processing (4)

• internal/buildapi/server.go
• internal/buildapi/server_upload_test.go
• internal/common/tasks/tasks.go
• internal/controller/imagebuild/controller.go