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
35 changes: 35 additions & 0 deletions framework/configstore/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,41 @@ func triggerMigrations(ctx context.Context, db *gorm.DB) error {
if err := migrationAddFeatureFlagsTable(ctx, db); err != nil {
return err
}
if err := migrationAddClientConfigMetadataColumn(ctx, db); err != nil {
return err
}
return nil
}

// migrationAddClientConfigMetadataColumn adds the metadata_json column to
// config_client. The column stores a JSON blob of UI/admin preferences (e.g.
// onboarding_dismissed) and is deliberately not part of the ClientConfig API
// struct, so config.json sync cannot overwrite it.
func migrationAddClientConfigMetadataColumn(ctx context.Context, db *gorm.DB) error {
m := migrator.New(db, migrator.DefaultOptions, []*migrator.Migration{{
ID: "add_client_config_metadata_json_column",
Migrate: func(tx *gorm.DB) error {
tx = tx.WithContext(ctx)
migrator := tx.Migrator()
if !migrator.HasColumn(&tables.TableClientConfig{}, "metadata_json") {
if err := migrator.AddColumn(&tables.TableClientConfig{}, "metadata_json"); err != nil {
return err
}
}
return nil
},
Rollback: func(tx *gorm.DB) error {
tx = tx.WithContext(ctx)
migrator := tx.Migrator()
if err := migrator.DropColumn(&tables.TableClientConfig{}, "metadata_json"); err != nil {
return err
}
return nil
},
}})
if err := m.Migrate(); err != nil {
return fmt.Errorf("error while running db migration: %s", err.Error())
}
return nil
}

Expand Down
85 changes: 84 additions & 1 deletion framework/configstore/rdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/bytedance/sonic"
bifrost "github.com/maximhq/bifrost/core"
providerUtils "github.com/maximhq/bifrost/core/providers/utils"
"github.com/maximhq/bifrost/core/schemas"
"github.com/maximhq/bifrost/framework/configstore/tables"
"github.com/maximhq/bifrost/framework/encrypt"
Expand Down Expand Up @@ -258,8 +259,18 @@ func (s *RDBConfigStore) UpdateClientConfig(ctx context.Context, config *ClientC
AllowPerRequestRawOverride: config.AllowPerRequestRawOverride,
ConfigHash: config.ConfigHash,
}
// Delete existing client config and create new one in a transaction
// Delete existing client config and create new one in a transaction.
// MetadataJSON is preserved here because Metadata is a UI/admin-preferences
// blob that is NOT part of the API-facing ClientConfig (so config.json sync
// can never set it). Reading it inside the transaction before DELETE keeps
// callers from clobbering UI prefs on every config write.
return s.DB().WithContext(ctx).Transaction(func(tx *gorm.DB) error {
var existing tables.TableClientConfig
if err := dbForUpdate(tx.Select("metadata_json")).First(&existing).Error; err == nil {
dbConfig.MetadataJSON = existing.MetadataJSON
} else if !errors.Is(err, gorm.ErrRecordNotFound) {
return err
}
if err := tx.Session(&gorm.Session{AllowGlobalUpdate: true}).Delete(&tables.TableClientConfig{}).Error; err != nil {
return err
}
Expand Down Expand Up @@ -513,6 +524,78 @@ func (s *RDBConfigStore) GetClientConfig(ctx context.Context) (*ClientConfig, er
}, nil
}

// GetClientMetadata returns the UI/admin-preferences blob stored on config_client.
// Returns an empty (non-nil) map if no row exists yet or the blob is unset, so
// callers can read keys without nil-checking.
func (s *RDBConfigStore) GetClientMetadata(ctx context.Context) (map[string]any, error) {
var dbConfig tables.TableClientConfig
if err := s.DB().WithContext(ctx).First(&dbConfig).Error; err != nil {
if errors.Is(err, gorm.ErrRecordNotFound) {
return map[string]any{}, nil
}
return nil, err
}
if dbConfig.Metadata == nil {
return map[string]any{}, nil
}
return dbConfig.Metadata, nil
}

// mergeMetadataPatch applies patch into dst following JSON Merge Patch
// semantics (RFC 7386): a nil patch value deletes the key; when both the
// existing value and the patch value are objects they are merged recursively;
// any other value replaces the existing one. dst is mutated in place.
func mergeMetadataPatch(dst, patch map[string]any) {
for k, v := range patch {
if v == nil {
delete(dst, k)
continue
}
patchObj, patchIsObj := v.(map[string]any)
dstObj, dstIsObj := dst[k].(map[string]any)
if patchIsObj && dstIsObj {
mergeMetadataPatch(dstObj, patchObj)
continue
}
dst[k] = v
}
}

// UpdateClientMetadata merges patch into the existing metadata blob and writes
// it back via a targeted UPDATE on metadata_json only — no DELETE+CREATE, no
// risk of clobbering other ClientConfig columns. The merge follows JSON Merge
// Patch semantics (RFC 7386): nested objects are merged recursively, and keys
// with a nil value in patch are removed from the blob (callers can pass
// {"key": nil} to clear, including nested keys).
func (s *RDBConfigStore) UpdateClientMetadata(ctx context.Context, patch map[string]any) error {
return s.DB().WithContext(ctx).Transaction(func(tx *gorm.DB) error {
var existing tables.TableClientConfig
if err := dbForUpdate(tx).First(&existing).Error; err != nil {
if errors.Is(err, gorm.ErrRecordNotFound) {
return fmt.Errorf("%w: client config must be initialized before metadata can be updated", ErrNotFound)
}
return err
}
merged := existing.Metadata
if merged == nil {
merged = map[string]any{}
}
mergeMetadataPatch(merged, patch)
data, mErr := providerUtils.MarshalSorted(merged)
if mErr != nil {
return mErr
}
result := tx.Model(&tables.TableClientConfig{}).Where("id = ?", existing.ID).Update("metadata_json", string(data))
if result.Error != nil {
return result.Error
}
if result.RowsAffected == 0 {
return fmt.Errorf("client config metadata update affected no rows")
}
return nil
})
}

// UpdateProvidersConfig updates the client configuration in the database.
func (s *RDBConfigStore) UpdateProvidersConfig(ctx context.Context, providers map[schemas.ModelProvider]ProviderConfig, tx ...*gorm.DB) error {
var txDB *gorm.DB
Expand Down
117 changes: 117 additions & 0 deletions framework/configstore/rdb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -872,6 +872,123 @@ func TestUpdateClientConfig(t *testing.T) {
assert.Equal(t, 100, result.InitialPoolSize)
}

func TestUpdateClientMetadata(t *testing.T) {
store := setupRDBTestStore(t)
ctx := context.Background()

err := store.UpdateClientConfig(ctx, &ClientConfig{
EnableLogging: new(true),
InitialPoolSize: 100,
LogRetentionDays: 30,
MaxRequestBodySizeMB: 50,
})
require.NoError(t, err)

err = store.UpdateClientMetadata(ctx, map[string]any{
"onboarding_dismissed": true,
"theme": "dark",
})
require.NoError(t, err)

err = store.UpdateClientMetadata(ctx, map[string]any{
"theme": "light",
"stale": nil,
})
require.NoError(t, err)

metadata, err := store.GetClientMetadata(ctx)
require.NoError(t, err)
assert.Equal(t, true, metadata["onboarding_dismissed"])
assert.Equal(t, "light", metadata["theme"])
assert.NotContains(t, metadata, "stale")

err = store.UpdateClientMetadata(ctx, map[string]any{"theme": nil})
require.NoError(t, err)

metadata, err = store.GetClientMetadata(ctx)
require.NoError(t, err)
assert.NotContains(t, metadata, "theme")
assert.Equal(t, true, metadata["onboarding_dismissed"])

// Nested objects must be merged recursively (RFC 7386), not replaced
// wholesale, so sibling keys survive a partial nested patch.
err = store.UpdateClientMetadata(ctx, map[string]any{
"onboarding": map[string]any{"dismissed": true, "step": "a"},
})
require.NoError(t, err)

err = store.UpdateClientMetadata(ctx, map[string]any{
"onboarding": map[string]any{"step": "b"},
})
require.NoError(t, err)

metadata, err = store.GetClientMetadata(ctx)
require.NoError(t, err)
onboarding, ok := metadata["onboarding"].(map[string]any)
require.True(t, ok)
assert.Equal(t, true, onboarding["dismissed"], "sibling key must survive nested patch")
assert.Equal(t, "b", onboarding["step"])

// A nil nested value deletes just that nested key.
err = store.UpdateClientMetadata(ctx, map[string]any{
"onboarding": map[string]any{"dismissed": nil},
})
require.NoError(t, err)

metadata, err = store.GetClientMetadata(ctx)
require.NoError(t, err)
onboarding, ok = metadata["onboarding"].(map[string]any)
require.True(t, ok)
assert.NotContains(t, onboarding, "dismissed")
assert.Equal(t, "b", onboarding["step"])
}

func TestUpdateClientMetadataRequiresClientConfig(t *testing.T) {
store := setupRDBTestStore(t)
ctx := context.Background()

err := store.UpdateClientMetadata(ctx, map[string]any{"onboarding_dismissed": true})
require.Error(t, err)
require.ErrorIs(t, err, ErrNotFound)

var count int64
err = store.DB().WithContext(ctx).Model(&tables.TableClientConfig{}).Count(&count).Error
require.NoError(t, err)
assert.Zero(t, count)
}

func TestUpdateClientConfigPreservesMetadata(t *testing.T) {
store := setupRDBTestStore(t)
ctx := context.Background()

err := store.UpdateClientConfig(ctx, &ClientConfig{
EnableLogging: new(true),
InitialPoolSize: 100,
LogRetentionDays: 30,
MaxRequestBodySizeMB: 50,
})
require.NoError(t, err)

err = store.UpdateClientMetadata(ctx, map[string]any{"onboarding_dismissed": true})
require.NoError(t, err)

err = store.UpdateClientConfig(ctx, &ClientConfig{
EnableLogging: new(true),
InitialPoolSize: 200,
LogRetentionDays: 60,
MaxRequestBodySizeMB: 100,
})
require.NoError(t, err)

config, err := store.GetClientConfig(ctx)
require.NoError(t, err)
assert.Equal(t, 200, config.InitialPoolSize)

metadata, err := store.GetClientMetadata(ctx)
require.NoError(t, err)
assert.Equal(t, true, metadata["onboarding_dismissed"])
}

// =============================================================================
// Transaction Tests
// =============================================================================
Expand Down
3 changes: 3 additions & 0 deletions framework/configstore/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ type ConfigStore interface {
// Client config CRUD
UpdateClientConfig(ctx context.Context, config *ClientConfig) error
GetClientConfig(ctx context.Context) (*ClientConfig, error)
// Client config metadata (UI/admin preferences blob — bypasses config.json sync)
GetClientMetadata(ctx context.Context) (map[string]any, error)
UpdateClientMetadata(ctx context.Context, patch map[string]any) error

// Framework config CRUD
UpdateFrameworkConfig(ctx context.Context, config *tables.TableFrameworkConfig) error
Expand Down
24 changes: 24 additions & 0 deletions framework/configstore/tables/clientconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type TableClientConfig struct {
AllowedOriginsJSON string `gorm:"type:text" json:"-"` // JSON serialized []string
AllowedHeadersJSON string `gorm:"type:text" json:"-"` // JSON serialized []string
HeaderFilterConfigJSON string `gorm:"type:text" json:"-"` // JSON serialized GlobalHeaderFilterConfig
MetadataJSON string `gorm:"type:text" json:"-"` // JSON serialized map[string]any for UI/admin preferences (e.g. onboarding_dismissed). Bypasses config.json sync.
InitialPoolSize int `gorm:"default:300" json:"initial_pool_size"`
EnableLogging *bool `gorm:"default:true" json:"enable_logging"`
DisableContentLogging bool `gorm:"default:false" json:"disable_content_logging"` // DisableContentLogging controls whether sensitive content (inputs, outputs, embeddings, etc.) is logged
Expand Down Expand Up @@ -61,6 +62,7 @@ type TableClientConfig struct {
LoggingHeaders []string `gorm:"-" json:"logging_headers,omitempty"`
WhitelistedRoutes []string `gorm:"-" json:"whitelisted_routes,omitempty"`
HeaderFilterConfig *GlobalHeaderFilterConfig `gorm:"-" json:"header_filter_config,omitempty"`
Metadata map[string]any `gorm:"-" json:"metadata,omitempty"`
}

// TableName sets the table name for each model
Expand Down Expand Up @@ -137,6 +139,18 @@ func (cc *TableClientConfig) BeforeSave(tx *gorm.DB) error {
cc.HeaderFilterConfigJSON = ""
}

// Metadata is preserved when nil — callers that DELETE+CREATE through
// UpdateClientConfig must carry MetadataJSON forward explicitly, since the
// API ClientConfig does not expose Metadata. A nil Metadata here means
// "leave whatever MetadataJSON the caller set untouched."
if cc.Metadata != nil {
data, err := json.Marshal(cc.Metadata)
if err != nil {
return err
}
cc.MetadataJSON = string(data)
}

return nil
}

Expand Down Expand Up @@ -186,5 +200,15 @@ func (cc *TableClientConfig) AfterFind(tx *gorm.DB) error {
cc.HeaderFilterConfig = &headerFilterConfig
}

if cc.MetadataJSON != "" {
var metadata map[string]any
if err := json.Unmarshal([]byte(cc.MetadataJSON), &metadata); err != nil {
return err
}
cc.Metadata = metadata
} else {
cc.Metadata = nil
}

return nil
}
34 changes: 34 additions & 0 deletions framework/configstore/tables/clientconfig_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package tables

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestTableClientConfigAfterFindReplacesMetadata(t *testing.T) {
config := &TableClientConfig{
MetadataJSON: `{"theme":"light"}`,
Metadata: map[string]any{
"stale": "value",
"theme": "dark",
},
}

require.NoError(t, config.AfterFind(nil))

assert.Equal(t, map[string]any{"theme": "light"}, config.Metadata)
}

func TestTableClientConfigAfterFindClearsMetadataWhenEmpty(t *testing.T) {
config := &TableClientConfig{
Metadata: map[string]any{
"stale": "value",
},
}

require.NoError(t, config.AfterFind(nil))

assert.Nil(t, config.Metadata)
}
6 changes: 6 additions & 0 deletions framework/configstore/tables/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ const (
ConfigHeaderFilterKey = "header_filter_config"
)

// Keys for the ClientConfig.MetadataJSON blob.
// These live inside the metadata JSON map on config_client, not as governance_config rows.
const (
MetadataKeyOnboardingDismissed = "onboarding_dismissed"
)

// RestartRequiredConfig represents the restart required configuration
// This is set when a config change requires a server restart to take effect
type RestartRequiredConfig struct {
Expand Down
Loading
Loading