unleash: Create dereferences a nil response when bifrost returns any non-201

Summary

unleash.Create never checks bifrost's HTTP status. The generated client returns err only for transport failures, so any non-201 response leaves resp.JSON201 nil, and the nil flows into toUnleashInstance, which dereferences it.

Severity: low today, rising — see "Why this has not bitten yet"
Component: internal/unleash/queries.go (Create), internal/unleash/models.go:221 and the toUnleashInstance below it

Mechanism

resp, err := client.CreateInstance(ctx, req)
if err != nil {
    return nil, err          // transport errors only
}
unleashInstance := bifrostUnleashToK8s(resp.JSON201)   // nil for any non-201
…
return toUnleashInstance(unleashInstance), nil          // dereferences u.Spec

bifrostUnleashToK8s guards nil and returns nil (models.go:222-224), but toUnleashInstance starts with for _, env := range u.Spec.ExtraEnvVars with no guard.

Reachable bifrost responses today: 400 validation_failed, 400 no_version_source, 500 creation_failed.

Why this has not bitten yet

Loki, 14 days of bifrost traffic: 2985 GET /v1/releasechannels, 15 PUT /v1/unleash/{name}, and zero POST /v1/unleash. Instance creation is essentially never exercised, so the path has not been hit. No matching panic appears in nais-api's logs over the same window.

Why it is worth fixing now

nais/bifrost#549 adds two new create responses — 409 already_exists when the instance already exists, and 503 when the existence check is inconclusive. The 409 in particular is raised on a duplicate POST, which is the one non-exotic way to hit a non-201. It converts a rare data-loss path in bifrost into a nil dereference here.

Proposed fix

Check the status before converting, and map bifrost's error responses to sensible GraphQL errors — at minimum 409 should surface as "an Unleash instance already exists for this team" rather than an internal error. A nil guard in toUnleashInstance is worth having regardless, as defence in depth.

Acceptance criteria

• A non-201 from bifrost returns an error, not a nil dereference.
• 409 surfaces a distinguishable "already exists" message.
• toUnleashInstance tolerates nil.
• Test covering at least the 409 and 500 responses.

Start in internal/unleash/queries.go at Create, then inspect bifrostUnleashToK8s and toUnleashInstance in internal/unleash/models.go. Add coverage for 409 and 500 responses, confirming non-201 responses return errors, 409 is distinguishable, and nil conversion does not panic.