caib: fix download when using --internal-registry

[bennyz]

Summary by CodeRabbit

• New Features

  • Hybrid registry builds: disk images may be pushed to the internal registry while container images are pushed externally.

• Improvements

  • Enhanced credential handling with token-first downloads and clearer validation for hybrid flows.
  • Updated CLI output/help text and auto-adjustment of build/export behavior for internal-registry scenarios.

• Bug Fixes

  • More predictable artifact downloads, including optional TLS-skip support for registry fetches and clearer fallback messaging.

[coderabbitai]

📝 Walkthrough

Walkthrough

Added TLS skip propagation and hybrid internal/external registry handling: CLI download/pull functions now accept an insecureSkipTLS flag; internal build API and server logic were updated to support disk pushes to the internal registry while optionally pushing container images externally, with adjusted credential flow and messaging.

Changes

Cohort / File(s)	Summary
CLI: caib main cmd/caib/main.go	Propagate insecureSkipTLS through OCI download/pull (downloadOCIArtifactIfRequested, pullOCIArtifact), adjust flags/help for --push-disk/--internal-registry, and prefer token-based downloads when RegistryToken is present.
Build API: OpenAPI internal/buildapi/openapi.yaml	Update BuildRequest.useInternalRegistry description to clarify it pushes disk images (mutually exclusive with exportOci; can combine with containerPush for hybrid builds).
Server: internal registry / hybrid flow internal/buildapi/server.go	Introduce hybrid internal/external registry handling: create external registry secret naming (-external-registry-auth), tighten validations for useInternalRegistry, conditionally generate internal container refs or preserve external containerPush, adjust ImageStream pre-creation, and return appropriate push/credential secret names for hybrid paths.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant CLI as CLI (caib)
participant BuildServer as Build API / server
participant InternalReg as Internal Registry
participant ExternalReg as External Registry
participant Storage as OCI Artifact Store

CLI->>BuildServer: submit build request (useInternalRegistry, containerPush?, RegistryToken?)
BuildServer->>BuildServer: validate hybrid rules, create secrets (internal / external)
BuildServer->>InternalReg: push disk image (if internal)
alt hybrid and external containerPush set
BuildServer->>ExternalReg: push container image
end
BuildServer->>Storage: publish OCI artifact (exportOci or internal store)
CLI->>Storage: download OCI artifact (token or creds + insecureSkipTLS)
Storage-->>CLI: deliver artifact

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• Add --insecure flag to caib CLI for testing with self-signed certificates #84 — Adds/propagates insecureSkipTLS option across CLI and push/download flows (closely related TLS propagation).
• introduce internal registry #81 — Related internal-registry feature work that adjusts flags and credential handling for hybrid builds.
• caib: restore caib download #79 — Prior changes to OCI artifact download/pull flow; connected to current download/credential adjustments.

Suggested reviewers

• bkhizgiy

Poem

"I hopped through lines of code last night,
Pushing disks where registries light,
Tokens tucked in carrot nooks,
TLS skipped with daring looks 🥕,
Now artifacts arrive — hooraay, delight!"

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 30.00% 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
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'caib: fix download when using --internal-registry' directly reflects the main changes in the changeset, which involve fixing download and artifact handling specifically for internal-registry scenarios.

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

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

No actionable comments were generated in the recent review. 🎉

---

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

🤖 Fix all issues with AI agents

In `@cmd/caib/main.go`:
- Around line 789-802: The OCI download call path doesn't propagate the global
--insecure flag; update the call site in main.go (where
downloadOCIArtifactIfRequested is invoked) to pass the insecureSkipTLS boolean
through, add an insecureSkipTLS parameter to downloadOCIArtifactIfRequested and
then forward it into pullOCIArtifact; inside pullOCIArtifact set the
SystemContext fields systemCtx.OCIInsecureSkipTLSVerify = insecureSkipTLS and
systemCtx.DockerInsecureSkipTLSVerify = types.OptionalBool(insecureSkipTLS) so
TLS verification is correctly disabled when insecureSkipTLS is true.

In `@internal/buildapi/server.go`:
- Around line 1183-1199: Rename the external registry secret created in the
hybrid path to avoid colliding with the internal registry secret: modify
createRegistrySecret (called in the hybrid branch after
setupInternalRegistryBuild) to generate a unique name (e.g., use
fmt.Sprintf("%s-external-registry-auth" or "%s-registry-auth-external") instead
of "%s-registry-auth") so it no longer matches createInternalRegistrySecret's
buildName + "-registry-auth"; ensure the hybrid branch continues to return the
new envSecretRef and that cleanup logic (which consumes the returned
envSecretRef) will correctly remove the externally-named secret.

🧹 Nitpick comments (1)

internal/buildapi/server.go (1)

1259-1269: Duplicate ImageStream entry for non-bootc builds.

For non-bootc builds with externalContainerPush == false, imageName is appended at both line 1262 and line 1268, resulting in ensureImageStream being called twice for the same name. This is harmless (the function is idempotent) but suggests the logic could be cleaner.

Suggested cleanup

 	// Pre-create ImageStream(s) for internal registry pushes only
 	var imageStreams []string
-	if !externalContainerPush {
-		imageStreams = append(imageStreams, imageName)
-	}
 	if req.Mode.IsBootc() && req.BuildDiskImage {
 		imageStreams = append(imageStreams, imageName+"-disk")
 	}
-	if !req.Mode.IsBootc() {
+	if !req.Mode.IsBootc() || !externalContainerPush {
 		imageStreams = append(imageStreams, imageName)
 	}

Wait — on second thought, the logic depends on what non-bootc + externalContainerPush even means. Since non-bootc modes don't use ContainerPush, externalContainerPush is always false for them, making the de-duplication safe. No action needed unless you want to tidy up.