feat(unleash): send the pre-shared key to bifrost - #499
Merged
Conversation
This was referenced Aug 18, 2026
Starefossen
force-pushed
the
feat/bifrost-psk-auth
branch
from
August 18, 2026 17:32
8012824 to
efdaf45
Compare
bifrost added pre-shared-key authentication (nais/bifrost#549) and is running in its accept phase: unauthenticated calls are allowed but logged as "Request without a valid API key allowed (authentication not enforced)". Every call nais-api makes is currently in that log. Sends the key as "Authorization: Bearer <key>" on every bifrost request, read from UNLEASH_BIFROST_API_KEY and provisioned through fasit to both deployments. An unset key sends no header at all rather than an empty one — a present-but-empty credential is worse than none, since it looks like a failed authentication rather than an unauthenticated caller. Startup logs a warning in that case, because it is the state that breaks the moment bifrost sets auth.enforced. This is step 3 of nais/bifrost#576. It must be deployed and confirmed before that flag is flipped; until then nothing changes behaviourally, since bifrost accepts both. Tests cover that the header is sent with the configured key and that no header is sent without one; the first fails if the request editor is removed.
The previous commit reads UNLEASH_BIFROST_API_KEY but nothing set it. The chart wires UNLEASH_BIFROST_API_URL and no key, so the client would have logged "No bifrost API key configured" and kept sending unauthenticated requests — the rollout would have looked done while changing nothing. Wires it end to end: the value is sourced from the fasit management value bifrost_api_key, lands in the release Secret alongside the other pre-shared keys, and reaches the container through the existing envFrom. Same pattern as HOOKD_PSK and REST_PRE_SHARED_KEY, which is where a secret belongs rather than a plain env value in the deployment.
The key is computed from the management value. It is deliberately not overridable per environment: staging the rollout is done with bifrost's enforcement toggle, not by varying who holds the credential. One key, one place it comes from. Refs nais/bifrost#576
Three review findings. The integration harness did not compile. api.ConfigureGraph gained a parameter and internal/integration/manager.go was not updated. The package sits behind //go:build integration_test, so go build ./... and go vet ./... both passed while CI — which runs go test -tags integration_test ./... — would have failed. Key rotation could not work. bifrost and nais-api read the same fasit value, and bifrost accepts a comma-separated list so keys can be rotated without downtime. nais-api sent the list verbatim, so a rotation value of "new,old" produced a credential matching neither key. Before enforcement that shows up as unauthenticated_allowed; after it, a full outage at the worst possible moment. The client now presents the first entry, so rotation is: set "new,old", let both sides settle, then drop the old one. The missing-key warning fired per request. NewBifrostClient is constructed per GraphQL request via the dataloader, so the warning would have emitted thousands of lines an hour in every environment until the key was provisioned. It is reported once at startup instead.
Starefossen
marked this pull request as ready for review
August 19, 2026 05:30
Starefossen
force-pushed
the
feat/bifrost-psk-auth
branch
from
August 19, 2026 05:30
c6cfc7d to
bc58f44
Compare
11 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
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.
Draft — not for merge yet. Part of the nais/bifrost#576 rollout.
The key now exists in fasit: nais/nais-terraform-modules#1561 is merged.
Context
bifrost added pre-shared-key authentication (nais/bifrost#549) and runs in its accept phase: unauthenticated calls are allowed but logged and counted. Verified in production:
nais-api is bifrost's only caller, so it is the gate on enforcement.
The chain
Feature.yaml→backend.auth.apiKeysFeature.yaml→unleash.bifrostApiKeysecret.yaml→UNLEASH_BIFROST_API_KEYvia existingenvFromAuthorization: Bearer <key>on every requestThe key lands in the release Secret rather than as a plain value in
deployment.yaml, alongsideHOOKD_PSKandREST_PRE_SHARED_KEY— same kind of value, same handling. It still reaches the container as an env var.Per-environment staging
unleash.bifrostApiKeydefaults to the management value but can be overridden or cleared per environment, following theaiven.tokenpattern in the same file. Clearing it sends no credential, which bifrost accepts until it enforces — so one environment can be taken out of the rollout without touching the others.Together with nais/bifrost#579, which makes
backend.auth.enforceda per-environment toggle, that is what allows dev to go first: send the key in dev, confirm the unauthenticated counter goes flat there, enforce in dev only, then repeat.Three commits
2deb514cthreads the key to both places a real client is constructed — the GraphQL dataloader and the issue checker — so no path is left unauthenticated. The fake client used locally and in tests is untouched.An unset key sends no header at all, not an empty one. A present-but-empty credential is worse than none: bifrost would record a failed authentication rather than an unauthenticated caller, which is harder to read during a rollout. Startup logs a warning when the key is missing, since that is the state that breaks when enforcement is flipped.
4c9309e9wires the chart. The first commit read the env var but nothing set it — the client would have logged "No bifrost API key configured" and kept sending unauthenticated requests, so the rollout would have looked complete while changing nothing.8012824badds the per-environment override described above.Behaviourally inert today
bifrost accepts both authenticated and unauthenticated calls, so merging this changes nothing user-visible. It only moves nais-api out of the warning log — the signal nais/bifrost#576 step 4 waits for.
Verification
go build ./...clean,internal/unleashtests pass. New tests assert the header is sent with the configured key and that no header is sent without one; the first fails if the request editor is removed. Chart renders with the key present;Feature.yamlparses.