Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions charts/nais-api/Feature.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,13 @@ values:
template: |
{{ .Management.bifrost_unleash_namespace | quote }}

unleash.bifrostApiKey:
displayName: Bifrost API pre-shared key
description: Pre-shared key authenticating nais-api to the bifrost API — the same value the bifrost deployment accepts.
computed:
template: |
{{ .Management.bifrost_api_key | quote }}

replaceEnvironmentNames:
displayName: Replace environment names
description: Mapping of environment names from current name to expected name. Format `currentName1:expectedName1,currentName2:expectedName2`
Expand Down
1 change: 1 addition & 0 deletions charts/nais-api/templates/secret.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ stringData:
DATABASE_URL: "postgres://{{ .Values.database.user }}:{{ .Values.database.password }}@127.0.0.1:5432/{{ .Values.database.name }}?sslmode=disable"
ZITADEL_KEY: {{ .Values.zitadel.key | quote }}
AIVEN_TOKEN: "{{ .Values.aiven.token }}"
UNLEASH_BIFROST_API_KEY: "{{ .Values.unleash.bifrostApiKey }}"
3 changes: 3 additions & 0 deletions charts/nais-api/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ unleash:
enabled: false
namespace: "bifrost-unleash"
bifrostApiUrl: "http://bifrost-backend"
# Pre-shared key for the bifrost API; provisioned in fasit as
# bifrost_api_key and shared with the bifrost deployment.
bifrostApiKey: ""

replicas: 2

Expand Down
9 changes: 8 additions & 1 deletion internal/cmd/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ func run(ctx context.Context, cfg *Config, log logrus.FieldLogger) error {
cfg.K8s.AllClusterNames(),
hookdClient,
cfg.Unleash.BifrostAPIURL,
cfg.Unleash.BifrostAPIKey,
cfg.K8s.AllClusterNames(),
cfg.Logging.DefaultLogDestinations(),
notifier,
Expand Down Expand Up @@ -428,7 +429,13 @@ func run(ctx context.Context, cfg *Config, log logrus.FieldLogger) error {
if cfg.Unleash.BifrostAPIURL == unleash.FakeBifrostURL {
bifrostClient = unleash.NewFakeBifrostClient(watchers.UnleashWatcher)
} else {
bifrostClient = unleash.NewBifrostClient(cfg.Unleash.BifrostAPIURL, log.WithField("subsystem", "bifrost_client"))
// Reported once here rather than in the constructor, which runs per
// request via the dataloader. This is the state that starts failing the
// moment bifrost enables enforcement.
if unleash.ActiveBifrostAPIKey(cfg.Unleash.BifrostAPIKey) == "" {
log.Warn("No bifrost API key configured; requests to bifrost will be unauthenticated and will be rejected once bifrost enforces authentication")
}
bifrostClient = unleash.NewBifrostClient(cfg.Unleash.BifrostAPIURL, cfg.Unleash.BifrostAPIKey, log.WithField("subsystem", "bifrost_client"))
}

issueChecker, err := checker.New(
Expand Down
4 changes: 4 additions & 0 deletions internal/cmd/api/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,10 @@ type oAuthConfig struct {
type unleashConfig struct {
// BifrostApiEndpoint is the endpoint for the Bifrost API
BifrostAPIURL string `env:"UNLEASH_BIFROST_API_URL,default=*fake*"`
// BifrostAPIKey is the pre-shared key sent to the Bifrost API. Provisioned
// via fasit to both deployments; empty means unauthenticated requests, which
// bifrost accepts only until it enables enforcement.
BifrostAPIKey string `env:"UNLEASH_BIFROST_API_KEY"`
}

type loggingConfig struct {
Expand Down
3 changes: 2 additions & 1 deletion internal/cmd/api/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ func ConfigureGraph(
clusters []string,
hookdClient hookd.Client,
bifrostAPIURL string,
bifrostAPIKey string,
allowedClusters []string,
defaultLogDestinations []logging.SupportedLogDestination,
notifier *notify.Notifier,
Expand Down Expand Up @@ -374,7 +375,7 @@ func ConfigureGraph(
ctx = serviceaccount.NewLoaderContext(ctx, pool)
ctx = session.NewLoaderContext(ctx, pool)
ctx = search.NewLoaderContext(ctx, pool, searcher)
ctx = unleash.NewLoaderContext(ctx, tenantName, watchers.UnleashWatcher, bifrostAPIURL, allowedClusters, log)
ctx = unleash.NewLoaderContext(ctx, tenantName, watchers.UnleashWatcher, bifrostAPIURL, bifrostAPIKey, allowedClusters, log)
ctx = tunnel.WithLoaders(ctx, tunnel.NewLoaders(watchers.TunnelWatcher))
ctx = logging.NewPackageContext(ctx, tenantName, defaultLogDestinations)
ctx = environment.NewLoaderContext(ctx, pool)
Expand Down
1 change: 1 addition & 0 deletions internal/integration/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ func newGQLRunner(
clusters(),
fakeHookd.New(),
unleash.FakeBifrostURL,
"", // bifrost API key: the harness uses the fake client, which needs none
[]string{"dev", "staging", "dev-fss", "dev-gcp"},
[]logging.SupportedLogDestination{logging.Loki},
notifier,
Expand Down
37 changes: 35 additions & 2 deletions internal/unleash/bifrost.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"net/http"
"strings"

"github.com/nais/bifrost/pkg/bifrostclient"
"github.com/sirupsen/logrus"
Expand All @@ -27,14 +28,46 @@ type bifrostClientImpl struct {
log logrus.FieldLogger
}

// ActiveBifrostAPIKey returns the key to present to bifrost, given the
// configured value.
//
// bifrost and nais-api read the same fasit value, and bifrost accepts a
// comma-separated list so keys can be rotated without downtime. A client must
// send exactly one of them, so the first entry is the active key. Rotation is
// therefore: set "new,old" (bifrost accepts both, nais-api presents new), then
// drop the old one. Sending the list verbatim would match nothing.
func ActiveBifrostAPIKey(configured string) string {
first, _, _ := strings.Cut(configured, ",")
return strings.TrimSpace(first)
}

// NewBifrostClient creates a new BifrostClient with the given base URL and logger.
// The client uses OpenTelemetry-instrumented HTTP transport for tracing.
func NewBifrostClient(baseURL string, log logrus.FieldLogger) BifrostClient {
//
// apiKey is the pre-shared key bifrost authenticates with, sent as
// "Authorization: Bearer <key>" on every request. An empty key sends no header
// at all — not an empty one — which bifrost accepts and counts during the
// accept-then-enforce phase. Once bifrost sets auth.enforced, an empty key
// means every call is rejected, so the key must be in place before that flip.
func NewBifrostClient(baseURL, apiKey string, log logrus.FieldLogger) BifrostClient {
httpClient := &http.Client{
Transport: otelhttp.NewTransport(http.DefaultTransport),
}

client, err := bifrostclient.NewClientWithResponses(baseURL, bifrostclient.WithHTTPClient(httpClient))
opts := []bifrostclient.ClientOption{bifrostclient.WithHTTPClient(httpClient)}
if key := ActiveBifrostAPIKey(apiKey); key != "" {
opts = append(opts, bifrostclient.WithRequestEditorFn(
func(ctx context.Context, req *http.Request) error {
req.Header.Set("Authorization", "Bearer "+key)
return nil
},
))
}
// No warning here: this constructor runs per request via the dataloader, so
// logging the missing-key state would emit a line on every GraphQL call.
// It is reported once at startup instead.

client, err := bifrostclient.NewClientWithResponses(baseURL, opts...)
if err != nil {
// This should only fail if the base URL is invalid
log.WithError(err).Fatal("failed to create bifrost client")
Expand Down
108 changes: 108 additions & 0 deletions internal/unleash/bifrost_auth_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
package unleash

import (
"context"
"io"
"net/http"
"net/http/httptest"
"testing"

"github.com/sirupsen/logrus"
)

// The pre-shared key must reach bifrost on every request. bifrost accepts
// "Authorization: Bearer <key>"; sending nothing is only tolerated until it
// enables enforcement, after which an unauthenticated call is a 401.
func TestNewBifrostClient_SendsPreSharedKey(t *testing.T) {
var gotAuth string
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
gotAuth = r.Header.Get("Authorization")
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(`[]`))
}))
t.Cleanup(srv.Close)

logger := logrus.New()
logger.SetOutput(io.Discard)

client := NewBifrostClient(srv.URL, "s3cret", logger)
if _, err := client.ListInstances(context.Background()); err != nil {
t.Fatalf("list: %v", err)
}

if want := "Bearer s3cret"; gotAuth != want {
t.Fatalf("Authorization header = %q, want %q", gotAuth, want)
}
}

// An unset key must not send an empty or malformed header — that would be worse
// than sending none, since bifrost would see a present-but-invalid credential.
func TestNewBifrostClient_NoKeySendsNoHeader(t *testing.T) {
var hadAuth bool
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
_, hadAuth = r.Header["Authorization"]
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(`[]`))
}))
t.Cleanup(srv.Close)

logger := logrus.New()
logger.SetOutput(io.Discard)

client := NewBifrostClient(srv.URL, "", logger)
if _, err := client.ListInstances(context.Background()); err != nil {
t.Fatalf("list: %v", err)
}

if hadAuth {
t.Fatal("no key configured, but an Authorization header was sent")
}
}

// bifrost and nais-api read the same fasit value, and bifrost accepts a
// comma-separated list so keys can be rotated without downtime. A client must
// present exactly one of them — sending the list verbatim matches nothing, and
// under enforcement that is a full outage at the worst possible moment.
func TestActiveBifrostAPIKey(t *testing.T) {
for _, tc := range []struct {
name, configured, want string
}{
{"single key", "abc123", "abc123"},
{"rotation: first is active", "new,old", "new"},
{"whitespace is trimmed", " new , old ", "new"},
{"empty stays empty", "", ""},
{"only separators is empty", " , ", ""},
} {
t.Run(tc.name, func(t *testing.T) {
if got := ActiveBifrostAPIKey(tc.configured); got != tc.want {
t.Fatalf("ActiveBifrostAPIKey(%q) = %q, want %q", tc.configured, got, tc.want)
}
})
}
}

// The whole point of picking one key: a rotation value must produce a header
// bifrost can actually match.
func TestNewBifrostClient_SendsOneKeyDuringRotation(t *testing.T) {
var gotAuth string
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
gotAuth = r.Header.Get("Authorization")
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(`[]`))
}))
t.Cleanup(srv.Close)

logger := logrus.New()
logger.SetOutput(io.Discard)

client := NewBifrostClient(srv.URL, "newkey,oldkey", logger)
if _, err := client.ListInstances(context.Background()); err != nil {
t.Fatalf("list: %v", err)
}

if want := "Bearer newkey"; gotAuth != want {
t.Fatalf("Authorization header = %q, want %q — the list must not be sent verbatim", gotAuth, want)
}
}
16 changes: 8 additions & 8 deletions internal/unleash/bifrost_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func TestBifrostClient_CreateInstance(t *testing.T) {
defer s.Close()

logger, _ := test.NewNullLogger()
bifrostClient := unleash.NewBifrostClient(s.URL, logger)
bifrostClient := unleash.NewBifrostClient(s.URL, "", logger)

name := "test"
allowedTeams := "team1,team2"
Expand Down Expand Up @@ -182,7 +182,7 @@ func TestBifrostClient_UpdateInstance(t *testing.T) {
defer s.Close()

logger, _ := test.NewNullLogger()
client := unleash.NewBifrostClient(s.URL, logger)
client := unleash.NewBifrostClient(s.URL, "", logger)

releaseChannel := "rapid"
req := bifrostclient.UnleashConfigRequest{
Expand Down Expand Up @@ -240,7 +240,7 @@ func TestBifrostClient_GetInstance(t *testing.T) {
defer s.Close()

logger, _ := test.NewNullLogger()
client := unleash.NewBifrostClient(s.URL, logger)
client := unleash.NewBifrostClient(s.URL, "", logger)

resp, err := client.GetInstance(context.Background(), "my-team")
if err != nil {
Expand Down Expand Up @@ -271,7 +271,7 @@ func TestBifrostClient_DeleteInstance(t *testing.T) {
defer s.Close()

logger, _ := test.NewNullLogger()
client := unleash.NewBifrostClient(s.URL, logger)
client := unleash.NewBifrostClient(s.URL, "", logger)

_, err := client.DeleteInstance(context.Background(), "my-team")
if err != nil {
Expand Down Expand Up @@ -322,7 +322,7 @@ func TestBifrostClient_ListChannels(t *testing.T) {
defer s.Close()

logger, _ := test.NewNullLogger()
client := unleash.NewBifrostClient(s.URL, logger)
client := unleash.NewBifrostClient(s.URL, "", logger)

resp, err := client.ListChannels(context.Background())
if err != nil {
Expand Down Expand Up @@ -382,7 +382,7 @@ func TestBifrostClient_ErrorHandling_CreateInstance(t *testing.T) {
defer s.Close()

logger, _ := test.NewNullLogger()
client := unleash.NewBifrostClient(s.URL, logger)
client := unleash.NewBifrostClient(s.URL, "", logger)

name := "test"
_, err := client.CreateInstance(context.Background(), bifrostclient.UnleashConfigRequest{
Expand Down Expand Up @@ -410,7 +410,7 @@ func TestBifrostClient_ErrorHandling_UpdateInstance(t *testing.T) {
defer s.Close()

logger, _ := test.NewNullLogger()
client := unleash.NewBifrostClient(s.URL, logger)
client := unleash.NewBifrostClient(s.URL, "", logger)

releaseChannel := "stable"
_, err := client.UpdateInstance(context.Background(), "my-team", bifrostclient.UnleashConfigRequest{
Expand All @@ -436,7 +436,7 @@ func TestBifrostClient_ErrorHandling_ListChannels(t *testing.T) {
defer s.Close()

logger, _ := test.NewNullLogger()
client := unleash.NewBifrostClient(s.URL, logger)
client := unleash.NewBifrostClient(s.URL, "", logger)

_, err := client.ListChannels(context.Background())

Expand Down
2 changes: 1 addition & 1 deletion internal/unleash/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func TestAllowedClustersMapping(t *testing.T) {
logger, _ := test.NewNullLogger()

// Create a bifrost client
client := NewBifrostClient(s.URL, logger)
client := NewBifrostClient(s.URL, "", logger)

// Simulate what happens in newLoaders function
mappedClusters := make([]string, len(tt.clusters))
Expand Down
8 changes: 4 additions & 4 deletions internal/unleash/dataloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ func (r mimirRoundTrip) RoundTrip(req *http.Request) (*http.Response, error) {

// NewLoaderContext creates a new context with a loaders value.
// If *fake* is provided as bifrostAPIURL, a fake client will be used.
func NewLoaderContext(ctx context.Context, tenantName string, appWatcher *watcher.Watcher[*UnleashInstance], bifrostAPIURL string, allowedClusters []string, log logrus.FieldLogger) context.Context {
return context.WithValue(ctx, loadersKey, newLoaders(tenantName, appWatcher, bifrostAPIURL, allowedClusters, log))
func NewLoaderContext(ctx context.Context, tenantName string, appWatcher *watcher.Watcher[*UnleashInstance], bifrostAPIURL, bifrostAPIKey string, allowedClusters []string, log logrus.FieldLogger) context.Context {
return context.WithValue(ctx, loadersKey, newLoaders(tenantName, appWatcher, bifrostAPIURL, bifrostAPIKey, allowedClusters, log))
}

func NewWatcher(ctx context.Context, mgr *watcher.Manager) *watcher.Watcher[*UnleashInstance] {
Expand All @@ -60,14 +60,14 @@ type loaders struct {
log logrus.FieldLogger
}

func newLoaders(tenantName string, appWatcher *watcher.Watcher[*UnleashInstance], bifrostAPIURL string, allowedClusters []string, log logrus.FieldLogger) *loaders {
func newLoaders(tenantName string, appWatcher *watcher.Watcher[*UnleashInstance], bifrostAPIURL, bifrostAPIKey string, allowedClusters []string, log logrus.FieldLogger) *loaders {
var client BifrostClient
var prometheus Prometheus
if bifrostAPIURL == FakeBifrostURL {
client = NewFakeBifrostClient(appWatcher)
prometheus = NewFakePrometheusClient()
} else {
client = NewBifrostClient(bifrostAPIURL, log)
client = NewBifrostClient(bifrostAPIURL, bifrostAPIKey, log)
promClient, err := promapi.NewClient(promapi.Config{Address: prometheusURL, RoundTripper: mimirRoundTrip{HeaderValue: "nais"}})
if err != nil {
panic(fmt.Errorf("failed to create prometheus client: %w", err))
Expand Down
Loading