fixes for jumpstarter integration

[bennyz]

• fail if there are no mappings
• only use internal registry route, not svc

Summary by CodeRabbit

• New Features

  • Adds an operator configuration endpoint that exposes available Jumpstarter targets.
  • Client now validates requested Jumpstarter targets against that configuration and returns clear error messages listing valid targets.

• Behavior

  • Builds using service-account registry auth will fast-fail with guidance if no external registry route is available; internal registry references are translated to the external route when applicable.

• Tests

  • Added coverage for the operator config endpoint and target mappings.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds an operator-config API and client, exposes Jumpstarter target mappings via /v1/config, validates requested Jumpstarter flash targets in CLI flows, and updates controller/reconciler to use an APIReader and adjust registry-route handling.

Changes

Cohort / File(s)	Summary
CLI validation cmd/caib/main.go	Add non-exported validateFlashTargetMapping(ctx, api, target) and call it from runBuild, runDisk, and runBuildDev to validate requested Jumpstarter targets before reading Jumpstarter client config.
API client internal/buildapi/client/client.go	Add (*Client) GetOperatorConfig(ctx) to GET /v1/config, validate 200, decode OperatorConfigResponse, and return it with consistent error wrapping.
API server & types internal/buildapi/server.go, internal/buildapi/types.go	Add /v1/config route and handleGetOperatorConfig handler that loads the OperatorConfig CR, constructs JumpstarterTargets map, and returns OperatorConfigResponse (new type).
API tests internal/buildapi/server_test.go	Add tests for /v1/config covering not-found (returns 200 with nil map) and present OperatorConfig (returns populated JumpstarterTargets), using DI stubs for client and loader.
Controller reconciler internal/controller/imagebuild/controller.go, cmd/main.go	Add APIReader client.Reader to ImageBuildReconciler; use APIReader as routeReader fallback, add fast-fail when service-account auth and no cluster registry route, and always translate internal registry ref to external route for flash flows when service-account auth.

Sequence Diagram

sequenceDiagram
    participant CLI as CLI (cmd/caib)
    participant Client as API Client (buildapi/client)
    participant Server as API Server (buildapi/server)
    participant K8s as Kubernetes API

    CLI->>Client: GetOperatorConfig(ctx)
    Client->>Server: GET /v1/config (auth)
    Server->>K8s: Read OperatorConfig "config" in namespace
    K8s-->>Server: OperatorConfig (or NotFound)
    Server->>Server: Build OperatorConfigResponse{JumpstarterTargets}
    Server-->>Client: 200 OperatorConfigResponse
    Client-->>CLI: Decoded JumpstarterTargets
    CLI->>CLI: validateFlashTargetMapping(target)
    alt target present
        CLI->>CLI: proceed with flash flow
    else target missing
        CLI->>CLI: error with available targets
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• initial jumpstarter integration #31: Adds Jumpstarter fields/types to OperatorConfig CRD that this API and CLI validation read and rely on.

Poem

🐰 I hopped to the config, ears alert and bright,
Found targets and maps in the soft morning light,
I checked every name with a twitch and a grin,
“Flash on!” I declared — let the tooling begin! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'fixes for jumpstarter integration' is vague and does not clearly convey the specific primary change. While it relates to the changeset, it lacks the specificity needed to understand the main intent from the PR history.	Revise the title to be more specific, such as 'Validate jumpstarter target mappings before flash operations' or 'Fail flash if jumpstarter target mapping is unavailable', to clearly communicate the primary change.

✅ Passed checks (1 passed)

Check name	Status	Explanation
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 docstrings

🧪 Generate unit tests (beta)

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

---

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 1291-1308: Extract the Jumpstarter target validation into a shared
helper function e.g. validateFlashTargetMapping(ctx, api, target) that calls
api.GetOperatorConfig(ctx), checks config.JumpstarterTargets is non-empty,
verifies the given target exists, and constructs the same availableTargets slice
before calling handleError with the existing error messages; then replace the
inline validation in runBuildDev with a call to validateFlashTargetMapping and
add calls to validateFlashTargetMapping at the flash-after-build branches in
runBuild and runDisk so all three paths (runBuild, runDisk, runBuildDev) perform
the same early validation using api.GetOperatorConfig and
config.JumpstarterTargets.

In `@internal/buildapi/client/client.go`:
- Line 402: The Authorization header is being set unconditionally
(req.Header.Set("Authorization", "Bearer "+c.authToken)) which can send an empty
token; wrap that call in the same guard used elsewhere (if c.authToken != "") so
the header is only added when a token exists—locate the unconditional
req.Header.Set call in client.go and change it to set the header only inside if
c.authToken != "" to match GetBuild/ListBuilds/CreateFlash patterns.

🧹 Nitpick comments (2)

cmd/caib/main.go (1)

1293-1293: Variable config shadows the imported config package.

Line 1293 declares config, err := api.GetOperatorConfig(ctx) which shadows the config package imported at Line 33. Rename to operatorCfg or opConfig to avoid confusion.

Suggested fix

-		config, err := api.GetOperatorConfig(ctx)
+		operatorCfg, err := api.GetOperatorConfig(ctx)

And update the references on lines 1298, 1302, 1303, 1304 accordingly.

internal/buildapi/server.go (1)

2408-2412: Consider distinguishing NotFound from other errors when fetching OperatorConfig.

If the OperatorConfig CR doesn't exist (common in fresh installations), this returns HTTP 500. A more helpful response would be to return 200 with an empty JumpstarterTargets (or 404), so the CLI can produce a clearer "no Jumpstarter target mappings configured" error rather than "failed to get operator configuration."

Suggested differentiation

 	if err != nil {
+		if k8serrors.IsNotFound(err) {
+			// No OperatorConfig exists - return empty config
+			c.JSON(http.StatusOK, OperatorConfigResponse{})
+			return
+		}
 		a.log.Error(err, "failed to get OperatorConfig", "reqID", reqID, "namespace", namespace)
 		c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to get operator configuration"})
 		return
 	}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@internal/buildapi/server.go`:
- Around line 2386-2425: In handleGetOperatorConfig, k8sClient.Get currently
treats a NotFound as a 500; change the error handling after k8sClient.Get to
detect a NotFound (using k8serrors.IsNotFound(err) / apierrors.IsNotFound) and
return an empty OperatorConfigResponse with HTTP 200 (or HTTP 404 if you prefer)
instead of an InternalServerError; update the error branch that logs "failed to
get OperatorConfig" to only log and return 500 for non-NotFound errors, and for
the NotFound case log at Info/Debug and respond with an empty response so
validateFlashTargetMapping can surface "no Jumpstarter target mappings
configured".

🧹 Nitpick comments (1)

cmd/caib/main.go (1)

718-742: Variable config shadows the imported config package.

On line 720, config shadows the config package imported at line 33 ("github.com/centos-automotive-suite/automotive-dev-operator/cmd/caib/config"). While it doesn't cause a bug here since the package isn't used within this function, it's a readability concern and could lead to subtle issues if the function is later extended.

Suggested rename

-func validateFlashTargetMapping(ctx context.Context, api *buildapiclient.Client, target string) {
-	config, err := api.GetOperatorConfig(ctx)
+func validateFlashTargetMapping(ctx context.Context, api *buildapiclient.Client, target string) {
+	operatorCfg, err := api.GetOperatorConfig(ctx)
 	if err != nil {
 		handleError(fmt.Errorf("failed to get operator configuration for Jumpstarter validation: %w", err))
 	}
 
-	if len(config.JumpstarterTargets) == 0 {
+	if len(operatorCfg.JumpstarterTargets) == 0 {
 		handleError(fmt.Errorf("flash enabled but no Jumpstarter target mappings configured in operator"))
 	}
 
-	if _, exists := config.JumpstarterTargets[target]; !exists {
-		availableTargets := make([]string, 0, len(config.JumpstarterTargets))
-		for t := range config.JumpstarterTargets {
+	if _, exists := operatorCfg.JumpstarterTargets[target]; !exists {
+		availableTargets := make([]string, 0, len(operatorCfg.JumpstarterTargets))
+		for t := range operatorCfg.JumpstarterTargets {
 			availableTargets = append(availableTargets, t)
 		}