diff --git a/contrib/cncf/technical-review.md b/contrib/cncf/technical-review.md index 2c252ecbca..8d680d1934 100644 --- a/contrib/cncf/technical-review.md +++ b/contrib/cncf/technical-review.md @@ -325,7 +325,7 @@ Default values can be found in [helm/kagent/values.yaml](https://github.com/kage **Additional Configurations:** For production use, configure: -- External PostgreSQL connection (set `database.postgres.bundled.enabled=false` and set either `database.postgres.url` or `database.postgres.urlFile`) +- External PostgreSQL connection (set `database.postgres.bundled.enabled=false` and configure `database.postgres.url` or `database.postgres.secretRef`) - LLM API keys via Secrets (`providers.openAI.apiKeySecretRef`) - TLS for external LLM connections (`modelConfig.tls`) - Resource limits based on workload (`agents.*.resources`) diff --git a/go/core/cli/internal/commands/db/db.go b/go/core/cli/internal/commands/db/db.go index a3ce67f12a..76054f5379 100644 --- a/go/core/cli/internal/commands/db/db.go +++ b/go/core/cli/internal/commands/db/db.go @@ -62,7 +62,7 @@ func migrationSources(namespace *string) dbmigrate.SourcesFunc { } else if b, ok := clusterVectorEnabled(ctx, *namespace); ok { vectorEnabled = b } - return migrations.BuiltinSources(vectorEnabled), nil + return migrations.BuiltinSourcesInSchema(vectorEnabled, kagentenv.DatabaseSchema.Get(), kagentenv.DatabaseVectorSchema.Get()), nil } } diff --git a/go/core/cli/internal/db/migrate/migrate.go b/go/core/cli/internal/db/migrate/migrate.go index 7a91718785..125a6f1dfd 100644 --- a/go/core/cli/internal/db/migrate/migrate.go +++ b/go/core/cli/internal/db/migrate/migrate.go @@ -22,6 +22,7 @@ import ( const ( dbURLEnv = "POSTGRES_DATABASE_URL" + dbRoleEnv = "POSTGRES_DATABASE_ROLE" sourceFlag = "source" ) @@ -35,6 +36,7 @@ type SourcesFunc func(ctx context.Context) ([]migrations.Source, error) type commandState struct { dbURL string + dbRole string source string resolveFn SourcesFunc @@ -93,9 +95,10 @@ func NewCommandFromFunc(fn SourcesFunc) *cobra.Command { Use: "migrate", Short: "Apply, roll back, and inspect database migrations", Long: `Apply, roll back, and inspect database migrations. -The command reads POSTGRES_DATABASE_URL when --db-url is empty.`, +The command reads POSTGRES_DATABASE_URL and POSTGRES_DATABASE_ROLE when their flags are empty.`, } command.PersistentFlags().StringVar(&state.dbURL, "db-url", "", "PostgreSQL connection URL") + command.PersistentFlags().StringVar(&state.dbRole, "db-role", "", "Stable PostgreSQL role to assume after authentication") command.PersistentFlags().StringVar(&state.source, sourceFlag, "", "Migration source for down, goto, or version") command.AddCommand(newUpCmd(state)) command.AddCommand(newDownCmd(state)) @@ -105,6 +108,13 @@ The command reads POSTGRES_DATABASE_URL when --db-url is empty.`, return command } +func (s *commandState) role() string { + if role := strings.TrimSpace(s.dbRole); role != "" { + return role + } + return strings.TrimSpace(os.Getenv(dbRoleEnv)) +} + func (s *commandState) resolveDSN() (string, error) { dsn := strings.TrimSpace(s.dbURL) if dsn == "" { @@ -204,7 +214,7 @@ func newUpCmd(state *commandState) *cobra.Command { if len(sources) == 0 { return errors.New("no migration sources are registered") } - if err := migrations.RunUp(command.Context(), dsn, sources); err != nil { + if err := migrations.RunUpAsRole(command.Context(), dsn, state.role(), sources); err != nil { return err } fmt.Fprintln(command.OutOrStdout(), "schema is up to date") @@ -236,7 +246,7 @@ func newDownCmd(state *commandState) *cobra.Command { if err != nil { return err } - return migrations.WithProvider(command.Context(), dsn, source, func(provider *goose.Provider) error { + return migrations.WithProviderAsRole(command.Context(), dsn, state.role(), source, func(provider *goose.Provider) error { current, err := readVersion(command.Context(), provider) if err != nil { return err @@ -316,7 +326,7 @@ func newStatusCmd(state *commandState) *cobra.Command { if err != nil { return err } - err = migrations.WithProvider(command.Context(), dsn, source, func(provider *goose.Provider) error { + err = migrations.WithProviderAsRole(command.Context(), dsn, state.role(), source, func(provider *goose.Provider) error { status, err := provider.Status(command.Context()) if err != nil { return err @@ -432,7 +442,7 @@ func newVersionCmd(state *commandState) *cobra.Command { sources = sources[index : index+1] } for _, source := range sources { - err := migrations.WithProvider(command.Context(), dsn, source, func(provider *goose.Provider) error { + err := migrations.WithProviderAsRole(command.Context(), dsn, state.role(), source, func(provider *goose.Provider) error { version, err := readVersion(command.Context(), provider) if err != nil { return err @@ -486,7 +496,7 @@ func newGotoCmd(state *commandState) *cobra.Command { if target != 0 && !slices.Contains(versions, target) { return fmt.Errorf("version %d is not available. Valid versions are %s", target, formatVersionList(versions)) } - return migrations.WithProvider(command.Context(), dsn, source, func(provider *goose.Provider) error { + return migrations.WithProviderAsRole(command.Context(), dsn, state.role(), source, func(provider *goose.Provider) error { current, err := readVersion(command.Context(), provider) if err != nil { return err diff --git a/go/core/cli/internal/db/migrate/migrate_test.go b/go/core/cli/internal/db/migrate/migrate_test.go index fd7a885f09..1ec4c37e1f 100644 --- a/go/core/cli/internal/db/migrate/migrate_test.go +++ b/go/core/cli/internal/db/migrate/migrate_test.go @@ -115,6 +115,16 @@ func TestResolveDSN(t *testing.T) { } } +func TestResolveRole(t *testing.T) { + t.Setenv(dbRoleEnv, "env_role") + if got := (&commandState{}).role(); got != "env_role" { + t.Fatalf("role() = %q, want env_role", got) + } + if got := (&commandState{dbRole: "flag_role"}).role(); got != "flag_role" { + t.Fatalf("role() = %q, want flag_role", got) + } +} + func TestResolveSource(t *testing.T) { multi := testSources() single := multi[:1] diff --git a/go/core/cmd/controller/main.go b/go/core/cmd/controller/main.go index 461f699a7d..3053c6c35a 100644 --- a/go/core/cmd/controller/main.go +++ b/go/core/cmd/controller/main.go @@ -22,9 +22,12 @@ import ( "log/slog" "os" "os/signal" + "strings" "syscall" + "github.com/kagent-dev/kagent/go/core/internal/database" "github.com/kagent-dev/kagent/go/core/pkg/app" + kagentenv "github.com/kagent-dev/kagent/go/core/pkg/env" ) func main() { @@ -35,6 +38,17 @@ func main() { logger := slog.Default() ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) defer stop() + switch os.Getenv("KAGENT_DATABASE_BOOTSTRAP") { + case "", "false": + case "true": + if err := runDatabaseBootstrap(ctx); err != nil { + logger.ErrorContext(ctx, "database bootstrap failed", "error", err) + os.Exit(1) + } + default: + logger.ErrorContext(ctx, "invalid database bootstrap value") + os.Exit(1) + } // No options: core's own controller runs with the default authenticator and // authorizer. A library consumer supplies its own by calling app.Run directly. @@ -43,3 +57,37 @@ func main() { os.Exit(1) } } + +func runDatabaseBootstrap(ctx context.Context) error { + adminUsername, err := readRequiredFile("POSTGRES_ADMIN_USERNAME_FILE") + if err != nil { + return err + } + adminPassword, err := readRequiredFile("POSTGRES_ADMIN_PASSWORD_FILE") + if err != nil { + return err + } + return database.Bootstrap(ctx, database.BootstrapConfig{ + EndpointSource: os.Getenv("POSTGRES_DATABASE_URL"), + AdminUsername: adminUsername, + AdminPassword: adminPassword, + Schema: kagentenv.DatabaseSchema.Get(), + VectorEnabled: kagentenv.DatabaseVectorEnabled.Get(), + VectorSchema: kagentenv.DatabaseVectorSchema.Get(), + }) +} + +func readRequiredFile(envName string) (string, error) { + path := os.Getenv(envName) + if path == "" { + return "", fmt.Errorf("%s must name a credential file", envName) + } + value, err := os.ReadFile(path) + if err != nil { + return "", fmt.Errorf("read %s: %w", envName, err) + } + if value := strings.TrimSpace(string(value)); value != "" { + return value, nil + } + return "", fmt.Errorf("%s credential file is empty", envName) +} diff --git a/go/core/internal/database/bootstrap.go b/go/core/internal/database/bootstrap.go new file mode 100644 index 0000000000..a5cb9879d4 --- /dev/null +++ b/go/core/internal/database/bootstrap.go @@ -0,0 +1,98 @@ +package database + +import ( + "context" + "errors" + "fmt" + + "github.com/jackc/pgx/v5" + "github.com/kagent-dev/kagent/go/core/pkg/migrations" +) + +const ( + OwnerRoleName = "kagent_owner" + UserName = "kagent_user" + UserPassword = "kagent" +) + +// BootstrapConfig contains the first-install PostgreSQL credentials. +// EndpointSource supplies the endpoint, database, and TLS configuration. +type BootstrapConfig struct { + EndpointSource string + AdminUsername string + AdminPassword string + Schema string + VectorEnabled bool + VectorSchema string +} + +// Bootstrap creates the fixed Kagent identity and schema. +// It does not change the password for an existing user. +func Bootstrap(ctx context.Context, cfg BootstrapConfig) error { + if cfg.EndpointSource == "" { + return errors.New("PostgreSQL connection string must not be empty") + } + if cfg.Schema == "" { + return errors.New("PostgreSQL schema must not be empty") + } + for name, value := range map[string]string{ + "administrator username": cfg.AdminUsername, + "administrator password": cfg.AdminPassword, + } { + if value == "" { + return fmt.Errorf("PostgreSQL %s must not be empty", name) + } + } + + dsn, err := ResolveURL(cfg.EndpointSource) + if err != nil { + return err + } + connConfig, err := pgx.ParseConfig(dsn) + if err != nil { + return errors.New("parse PostgreSQL bootstrap connection string: invalid value") + } + if connConfig.User != UserName { + return fmt.Errorf("PostgreSQL bootstrap connection string must contain the %q user", UserName) + } + if connConfig.Password != UserPassword { + return errors.New("PostgreSQL bootstrap connection string does not match the fixed development password") + } + connConfig.User = cfg.AdminUsername + connConfig.Password = cfg.AdminPassword + conn, err := pgx.ConnectConfig(ctx, connConfig) + if err != nil { + return fmt.Errorf("connect as PostgreSQL administrator: %w", err) + } + defer conn.Close(ctx) //nolint:errcheck // The transaction result decides success. + + tx, err := conn.Begin(ctx) + if err != nil { + return fmt.Errorf("start PostgreSQL bootstrap transaction: %w", err) + } + defer tx.Rollback(ctx) //nolint:errcheck // Commit or the returned error decides the outcome. + + for setting, value := range map[string]string{ + "kagent.bootstrap_username": UserName, + "kagent.bootstrap_password": UserPassword, + "kagent.bootstrap_owner_role": OwnerRoleName, + "kagent.bootstrap_schema": cfg.Schema, + "kagent.bootstrap_vector_enabled": fmt.Sprint(cfg.VectorEnabled), + "kagent.bootstrap_vector_schema": cfg.VectorSchema, + } { + if _, err := tx.Exec(ctx, `SELECT set_config($1, $2, true)`, setting, value); err != nil { + return fmt.Errorf("set PostgreSQL bootstrap parameter %q: %w", setting, err) + } + } + identitySQL, err := migrations.FS.ReadFile("identity/bootstrap.sql") + if err != nil { + return fmt.Errorf("read PostgreSQL identity SQL: %w", err) + } + if _, err := tx.Conn().PgConn().ExecParams(ctx, string(identitySQL), nil, nil, nil, nil).Close(); err != nil { + return fmt.Errorf("apply PostgreSQL identity SQL: %w", err) + } + if err := tx.Commit(ctx); err != nil { + return fmt.Errorf("commit PostgreSQL bootstrap: %w", err) + } + return nil +} diff --git a/go/core/internal/database/bootstrap_test.go b/go/core/internal/database/bootstrap_test.go new file mode 100644 index 0000000000..c638daac58 --- /dev/null +++ b/go/core/internal/database/bootstrap_test.go @@ -0,0 +1,283 @@ +package database + +import ( + "context" + "fmt" + "net/url" + "testing" + + "github.com/jackc/pgx/v5" + "github.com/kagent-dev/kagent/go/core/pkg/migrations" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestBootstrapCreatesManagedIdentity(t *testing.T) { + const schema = "kagent_bootstrap_test" + cleanupBootstrap(t, schema) + t.Cleanup(func() { cleanupBootstrap(t, schema) }) + + adminConfig, err := pgx.ParseConfig(sharedConnStr) + require.NoError(t, err) + dsn, err := url.Parse(sharedConnStr) + require.NoError(t, err) + dsn.User = url.UserPassword(UserName, UserPassword) + cfg := BootstrapConfig{ + EndpointSource: dsn.String(), + AdminUsername: adminConfig.User, + AdminPassword: adminConfig.Password, + Schema: schema, + } + errs := make(chan error, 2) + for range 2 { + go func() { errs <- Bootstrap(t.Context(), cfg) }() + } + for range 2 { + require.NoError(t, <-errs) + } + + var owner string + require.NoError(t, sharedDB.QueryRow(t.Context(), + `SELECT pg_get_userbyid(nspowner) FROM pg_namespace WHERE nspname = $1`, schema).Scan(&owner)) + assert.Equal(t, OwnerRoleName, owner) + + var member bool + require.NoError(t, sharedDB.QueryRow(t.Context(), + `SELECT pg_has_role($1, $2, 'MEMBER')`, UserName, OwnerRoleName).Scan(&member)) + assert.True(t, member) + + // A retry validates the identity. It must not reset a password changed by an operator. + _, err = sharedDB.Exec(t.Context(), `ALTER ROLE kagent_user PASSWORD 'replacement-password'`) + require.NoError(t, err) + require.NoError(t, Bootstrap(t.Context(), BootstrapConfig{ + EndpointSource: dsn.String(), + AdminUsername: adminConfig.User, + AdminPassword: adminConfig.Password, + Schema: schema, + })) + + rotated := *dsn + rotated.User = url.UserPassword(UserName, "replacement-password") + conn, err := pgx.Connect(t.Context(), rotated.String()) + require.NoError(t, err) + defer conn.Close(t.Context()) //nolint:errcheck + _, err = conn.Exec(t.Context(), "SET ROLE "+pgx.Identifier{OwnerRoleName}.Sanitize()) + require.NoError(t, err) + _, err = conn.Exec(t.Context(), fmt.Sprintf(`CREATE TABLE %s.bootstrap_data (id integer)`, + pgx.Identifier{schema}.Sanitize())) + require.NoError(t, err) + + _, err = sharedDB.Exec(t.Context(), ` + DROP SCHEMA IF EXISTS substrate_isolation_test CASCADE; + CREATE SCHEMA substrate_isolation_test; + REVOKE ALL ON SCHEMA substrate_isolation_test FROM PUBLIC; + CREATE TABLE substrate_isolation_test.private_data (id integer)`) + require.NoError(t, err) + t.Cleanup(func() { + _, _ = sharedDB.Exec(context.Background(), `DROP SCHEMA IF EXISTS substrate_isolation_test CASCADE`) + }) + _, err = conn.Exec(t.Context(), `SELECT * FROM substrate_isolation_test.private_data`) + require.Error(t, err) +} + +func TestBootstrapAndMigrateInPublicSchema(t *testing.T) { + const databaseName = "kagent_public_bootstrap_test" + _, err := sharedDB.Exec(t.Context(), "CREATE DATABASE "+databaseName) + require.NoError(t, err) + t.Cleanup(func() { + _, err := sharedDB.Exec(context.Background(), "DROP DATABASE "+databaseName+" WITH (FORCE)") + require.NoError(t, err) + _, err = sharedDB.Exec(context.Background(), "DROP ROLE IF EXISTS "+UserName) + require.NoError(t, err) + _, err = sharedDB.Exec(context.Background(), "DROP ROLE IF EXISTS "+OwnerRoleName) + require.NoError(t, err) + }) + + dsn, err := url.Parse(sharedConnStr) + require.NoError(t, err) + dsn.Path = "/" + databaseName + adminConfig, err := pgx.ParseConfig(dsn.String()) + require.NoError(t, err) + dsn.User = url.UserPassword(UserName, UserPassword) + require.NoError(t, Bootstrap(t.Context(), BootstrapConfig{ + EndpointSource: dsn.String(), + AdminUsername: adminConfig.User, + AdminPassword: adminConfig.Password, + Schema: "public", + })) + require.NoError(t, migrations.RunUpAsRole(t.Context(), dsn.String(), OwnerRoleName, + migrations.BuiltinSourcesInSchema(false, "public", "public"))) + + conn, err := pgx.Connect(t.Context(), dsn.String()) + require.NoError(t, err) + defer conn.Close(t.Context()) //nolint:errcheck + _, err = conn.Exec(t.Context(), "SET ROLE "+OwnerRoleName) + require.NoError(t, err) + var migrated bool + require.NoError(t, conn.QueryRow(t.Context(), + "SELECT to_regclass('public.schema_migrations') IS NOT NULL").Scan(&migrated)) + require.True(t, migrated) +} + +func TestBootstrapSharesPgvectorAcrossSchemas(t *testing.T) { + const databaseName = "kagent_shared_vector_test" + _, err := sharedDB.Exec(t.Context(), "CREATE DATABASE "+databaseName) + require.NoError(t, err) + t.Cleanup(func() { + _, err := sharedDB.Exec(context.Background(), "DROP DATABASE "+databaseName+" WITH (FORCE)") + require.NoError(t, err) + _, err = sharedDB.Exec(context.Background(), "DROP ROLE IF EXISTS "+UserName) + require.NoError(t, err) + _, err = sharedDB.Exec(context.Background(), "DROP ROLE IF EXISTS "+OwnerRoleName) + require.NoError(t, err) + }) + + adminDSN, err := url.Parse(sharedConnStr) + require.NoError(t, err) + adminDSN.Path = "/" + databaseName + adminConfig, err := pgx.ParseConfig(adminDSN.String()) + require.NoError(t, err) + appDSN := *adminDSN + appDSN.User = url.UserPassword(UserName, UserPassword) + for _, schema := range []string{"tenant_one", "tenant_two"} { + require.NoError(t, Bootstrap(t.Context(), BootstrapConfig{ + EndpointSource: appDSN.String(), + AdminUsername: adminConfig.User, + AdminPassword: adminConfig.Password, + Schema: schema, + VectorEnabled: true, + })) + require.NoError(t, migrations.RunUpAsRole(t.Context(), appDSN.String(), OwnerRoleName, + migrations.BuiltinSourcesInSchema(true, schema, "extensions"))) + } + + conn, err := pgx.Connect(t.Context(), adminDSN.String()) + require.NoError(t, err) + defer conn.Close(t.Context()) //nolint:errcheck + var vectorSchema string + require.NoError(t, conn.QueryRow(t.Context(), `SELECT n.nspname FROM pg_extension e JOIN pg_namespace n ON n.oid = e.extnamespace WHERE e.extname = 'vector'`).Scan(&vectorSchema)) + assert.Equal(t, "extensions", vectorSchema) + for _, schema := range []string{"tenant_one", "tenant_two"} { + var exists bool + require.NoError(t, conn.QueryRow(t.Context(), "SELECT to_regclass($1) IS NOT NULL", schema+".memory").Scan(&exists)) + assert.True(t, exists, "memory table in %s", schema) + } + wrongSchema := Bootstrap(t.Context(), BootstrapConfig{ + EndpointSource: appDSN.String(), AdminUsername: adminConfig.User, AdminPassword: adminConfig.Password, + Schema: "tenant_three", VectorEnabled: true, VectorSchema: "public", + }) + require.ErrorContains(t, wrongSchema, `pgvector is installed in schema "extensions", expected "public"`) + var thirdSchemaExists bool + require.NoError(t, conn.QueryRow(t.Context(), "SELECT EXISTS (SELECT 1 FROM pg_namespace WHERE nspname = 'tenant_three')").Scan(&thirdSchemaExists)) + assert.False(t, thirdSchemaExists) + + pool, err := Connect(t.Context(), &PostgresConfig{ + URL: appDSN.String(), Role: OwnerRoleName, Schema: "tenant_two", VectorEnabled: true, + }) + require.NoError(t, err) + defer pool.Close() + var currentSchema string + require.NoError(t, pool.QueryRow(t.Context(), "SELECT current_schema()").Scan(¤tSchema)) + assert.Equal(t, "tenant_two", currentSchema) + var searchPath string + require.NoError(t, pool.QueryRow(t.Context(), "SHOW search_path").Scan(&searchPath)) + assert.Equal(t, `"tenant_two"`, searchPath) + client := NewClient(pool, "extensions") + memory := &Memory{AgentName: "agent", UserID: "user", Content: "test", Embedding: makeEmbedding(1)} + require.NoError(t, client.StoreAgentMemories(t.Context(), memory)) + results, err := client.SearchAgentMemory(t.Context(), "agent", "user", makeEmbedding(1), 1) + require.NoError(t, err) + require.Len(t, results, 1) + assert.Equal(t, memory.ID, results[0].ID) +} + +func TestBootstrapRequiresConnectionString(t *testing.T) { + err := Bootstrap(t.Context(), BootstrapConfig{}) + require.ErrorContains(t, err, "connection string must not be empty") +} + +func TestBootstrapRejectsCustomLogin(t *testing.T) { + err := Bootstrap(t.Context(), BootstrapConfig{ + EndpointSource: "postgresql://custom:password@localhost/kagent", + AdminUsername: "postgres", + AdminPassword: "postgres", + Schema: "kagent", + }) + require.ErrorContains(t, err, `must contain the "kagent_user" user`) +} + +func TestBootstrapRejectsCustomPassword(t *testing.T) { + err := Bootstrap(t.Context(), BootstrapConfig{ + EndpointSource: "postgresql://kagent_user:custom@localhost/kagent", + AdminUsername: "postgres", + AdminPassword: "postgres", + Schema: "public", + }) + require.ErrorContains(t, err, "does not match the fixed development password") +} + +func TestIdentitySQLSupportsOperatorLogin(t *testing.T) { + const ( + schema = "kagent_operator_identity_test" + username = "operator's-login" + password = "operator's-password" + role = "operator's-owner" + ) + cleanupBootstrap(t, schema) + t.Cleanup(func() { cleanupBootstrap(t, schema) }) + t.Cleanup(func() { + _, err := sharedDB.Exec(context.Background(), fmt.Sprintf(` + DROP SCHEMA IF EXISTS %s CASCADE; + DROP ROLE IF EXISTS %s; + DROP ROLE IF EXISTS %s`, + pgx.Identifier{schema}.Sanitize(), + pgx.Identifier{username}.Sanitize(), + pgx.Identifier{role}.Sanitize())) + require.NoError(t, err) + }) + + identitySQL, err := migrations.FS.ReadFile("identity/bootstrap.sql") + require.NoError(t, err) + tx, err := sharedDB.Begin(t.Context()) + require.NoError(t, err) + defer tx.Rollback(t.Context()) //nolint:errcheck + for setting, value := range map[string]string{ + "kagent.bootstrap_username": username, + "kagent.bootstrap_password": password, + "kagent.bootstrap_schema": schema, + "kagent.bootstrap_owner_role": role, + "kagent.bootstrap_vector_enabled": "false", + } { + _, err := tx.Exec(t.Context(), `SELECT set_config($1, $2, true)`, setting, value) + require.NoError(t, err) + } + _, err = tx.Exec(t.Context(), string(identitySQL)) + require.NoError(t, err) + require.NoError(t, tx.Commit(t.Context())) + var schemaOwner string + require.NoError(t, sharedDB.QueryRow(t.Context(), + `SELECT pg_get_userbyid(nspowner) FROM pg_namespace WHERE nspname = $1`, schema).Scan(&schemaOwner)) + assert.Equal(t, role, schemaOwner) + + connConfig, err := pgx.ParseConfig(sharedConnStr) + require.NoError(t, err) + connConfig.User, connConfig.Password = username, password + conn, err := pgx.ConnectConfig(t.Context(), connConfig) + require.NoError(t, err) + defer conn.Close(t.Context()) //nolint:errcheck + _, err = conn.Exec(t.Context(), "SET ROLE "+pgx.Identifier{role}.Sanitize()) + require.NoError(t, err) +} + +func cleanupBootstrap(t *testing.T, schema string) { + t.Helper() + ctx := context.Background() + _, err := sharedDB.Exec(ctx, fmt.Sprintf(` + DROP SCHEMA IF EXISTS %s CASCADE; + DROP ROLE IF EXISTS %s; + DROP ROLE IF EXISTS %s`, + pgx.Identifier{schema}.Sanitize(), + pgx.Identifier{UserName}.Sanitize(), + pgx.Identifier{OwnerRoleName}.Sanitize())) + require.NoError(t, err) +} diff --git a/go/core/internal/database/client_postgres.go b/go/core/internal/database/client_postgres.go index c522f3c299..c834b14db5 100644 --- a/go/core/internal/database/client_postgres.go +++ b/go/core/internal/database/client_postgres.go @@ -7,18 +7,24 @@ import ( "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" + "github.com/kagent-dev/kagent/go/core/pkg/consts" ) // Client persists control-plane state in PostgreSQL. Callers define the narrow // interfaces they need; SQL rows and protobuf encoding stay inside the store. type Client struct { - db *pgxpool.Pool + db *pgxpool.Pool + vectorCosineOperator string } // NewClient wraps an existing PostgreSQL pool without connecting or migrating. The caller -// owns the pool and must close it. -func NewClient(db *pgxpool.Pool) *Client { - return &Client{db: db} +// owns the pool and must close it. The optional pgvector schema defaults to extensions. +func NewClient(db *pgxpool.Pool, vectorSchema ...string) *Client { + schema := consts.DefaultPgvectorSchema + if len(vectorSchema) > 0 && vectorSchema[0] != "" { + schema = vectorSchema[0] + } + return &Client{db: db, vectorCosineOperator: "OPERATOR(" + pgx.Identifier{schema}.Sanitize() + ".<=>)"} } // withTx commits all callback writes together on success and rolls them back on failure, diff --git a/go/core/internal/database/client_test.go b/go/core/internal/database/client_test.go index 1cfbe4ec73..9c4f6b88dd 100644 --- a/go/core/internal/database/client_test.go +++ b/go/core/internal/database/client_test.go @@ -14,12 +14,17 @@ import ( "github.com/stretchr/testify/require" ) +func TestClientVectorSchemaDefaultAndOverride(t *testing.T) { + assert.Equal(t, `OPERATOR("extensions".<=>)`, NewClient(nil).vectorCosineOperator) + assert.Equal(t, `OPERATOR("public".<=>)`, NewClient(nil, "public").vectorCosineOperator) +} + // TestDirectModelScans covers database defaults, required catalog fields, and nullable // memory fields when rows are scanned directly into application models. func TestDirectModelScans(t *testing.T) { ctx := t.Context() db := setupTestDB(t) - client := NewClient(db) + client := NewClient(db, "public") _, err := db.Exec(ctx, `INSERT INTO tool (id, server_name, group_kind) VALUES ('defaulted', 'server', 'kind')`) require.NoError(t, err) _, err = db.Exec(ctx, `INSERT INTO toolserver (name, group_kind) VALUES ('defaulted', 'kind')`) @@ -104,9 +109,8 @@ func setupTestDB(t *testing.T) *pgxpool.Pool { t.Skip("skipping database test in short mode") } - // Truncate application tables instead of full down+up migrations. - // Full down migration drops and recreates the pgvector extension, which - // changes type OIDs and breaks existing pool connections. + // Truncate application tables instead of rebuilding the schema while + // shared pool connections are active. _, err := sharedDB.Exec(context.Background(), ` TRUNCATE TABLE scheduled_run, @@ -134,7 +138,7 @@ func makeEmbedding(v float32) pgvector.Vector { // via vector similarity search and that results are ordered by cosine similarity. func TestStoreAndSearchAgentMemory(t *testing.T) { db := setupTestDB(t) - client := NewClient(db) + client := NewClient(db, "public") ctx := context.Background() agentName := "test-agent" @@ -183,7 +187,7 @@ func TestStoreAndSearchAgentMemory(t *testing.T) { // atomically via a transaction and that they are all retrievable afterwards. func TestStoreAgentMemoriesBatch(t *testing.T) { db := setupTestDB(t) - client := NewClient(db) + client := NewClient(db, "public") ctx := context.Background() agentName := "batch-agent" @@ -207,7 +211,7 @@ func TestStoreAgentMemoriesBatch(t *testing.T) { // searching for similar memories. func TestSearchAgentMemoryLimit(t *testing.T) { db := setupTestDB(t) - client := NewClient(db) + client := NewClient(db, "public") ctx := context.Background() agentName := "limit-agent" @@ -247,7 +251,7 @@ func TestSearchAgentMemoryLimit(t *testing.T) { // correct (agentName, userID) pair and do not return results for other agents or users. func TestSearchAgentMemoryIsolation(t *testing.T) { db := setupTestDB(t) - client := NewClient(db) + client := NewClient(db, "public") ctx := context.Background() mem1 := &Memory{AgentName: "agent-a", UserID: "user-1", Content: "agent-a user-1 memory", Embedding: makeEmbedding(0.5)} @@ -266,7 +270,7 @@ func TestSearchAgentMemoryIsolation(t *testing.T) { // normalization ListAgentMemories and DeleteAgentMemory already apply. func TestSearchAgentMemoryNormalizedName(t *testing.T) { db := setupTestDB(t) - client := NewClient(db) + client := NewClient(db, "public") ctx := context.Background() stored := &Memory{AgentName: "ns__my_agent", UserID: "user-1", Content: "stored under underscore form", Embedding: makeEmbedding(0.5)} @@ -282,7 +286,7 @@ func TestSearchAgentMemoryNormalizedName(t *testing.T) { // given agent/user pair and that the hyphen-to-underscore normalization works correctly. func TestDeleteAgentMemory(t *testing.T) { db := setupTestDB(t) - client := NewClient(db) + client := NewClient(db, "public") ctx := context.Background() agentName := "my-agent" @@ -316,7 +320,7 @@ func TestDeleteAgentMemory(t *testing.T) { // and that frequently-accessed expired memories have their TTL extended instead. func TestPruneExpiredMemories(t *testing.T) { db := setupTestDB(t) - client := NewClient(db) + client := NewClient(db, "public") ctx := context.Background() agentName := "prune-agent" @@ -365,7 +369,7 @@ func countRows(t *testing.T, db *pgxpool.Pool, query string, args ...any) int64 // return results. func TestSearchAgentMemoryConcurrentAccessCount(t *testing.T) { db := setupTestDB(t) - client := NewClient(db) + client := NewClient(db, "public") ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) t.Cleanup(cancel) diff --git a/go/core/internal/database/connect.go b/go/core/internal/database/connect.go index be0c0fa042..db6d056549 100644 --- a/go/core/internal/database/connect.go +++ b/go/core/internal/database/connect.go @@ -2,25 +2,33 @@ package database import ( "context" + "errors" "fmt" "os" + "path/filepath" + "slices" "strings" "time" "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" "github.com/jackc/pgx/v5/pgxpool" + "github.com/kagent-dev/kagent/go/core/pkg/consts" "github.com/kagent-dev/kagent/go/pkg/logging" pgvectorpgx "github.com/pgvector/pgvector-go/pgx" ) // PostgresConfig holds the connection parameters for a Postgres database. -// URL must be a resolved connection string — use ResolveURL to resolve from -// a file path before constructing this config. -// +// URL is either a literal connection string or @file:/absolute/path. File +// sources are reread for every new physical connection. // Pool fields are optional: nil leaves the corresponding pgxpool.Config value // from ParseConfig unchanged (pgx library defaults). +// Schema is required when vectors are enabled; VectorSchema defaults to extensions. type PostgresConfig struct { URL string + Role string + Schema string + VectorSchema string VectorEnabled bool MaxConns *int32 MinConns *int32 @@ -32,8 +40,11 @@ const ( defaultMaxTimeout = 120 * time.Second defaultInitialDelay = 500 * time.Millisecond defaultMaxDelay = 5 * time.Second + fileSourcePrefix = "@file:" ) +var errInvalidDatabaseURL = errors.New("invalid PostgreSQL connection string") + // Connect returns a PostgreSQL pool after a successful ping, retrying until the // context is canceled or two minutes elapse. Invalid configuration fails immediately. // VectorEnabled registers pgvector types on each connection. The caller closes the pool. @@ -65,6 +76,167 @@ func applyPoolConfig(config *pgxpool.Config, cfg *PostgresConfig) error { return nil } +// ResolveURL resolves a literal or file-backed database URL for one-time uses +// such as startup migrations. Connect retains the source expression so future +// physical connections can refresh file-backed credentials. +func ResolveURL(source string) (string, error) { + url, err := resolveURL(source) + if err != nil { + return "", err + } + if _, err := parsePoolConfig(url); err != nil { + return "", err + } + return url, nil +} + +func resolveURL(source string) (string, error) { + path, fileBacked := strings.CutPrefix(source, fileSourcePrefix) + if !fileBacked { + return source, nil + } + if !filepath.IsAbs(path) { + return "", fmt.Errorf("database connection source path %q must be absolute", path) + } + content, err := os.ReadFile(path) + if err != nil { + return "", fmt.Errorf("read database connection source %s: %w", path, err) + } + url := strings.TrimSpace(string(content)) + if url == "" { + return "", fmt.Errorf("database connection source %s is empty", path) + } + return url, nil +} + +func parsePoolConfig(url string) (*pgxpool.Config, error) { + config, err := pgxpool.ParseConfig(url) + if err != nil { + // pgx parse errors retain the input string, so wrapping err here could + // disclose the password read from a Secret. + return nil, fmt.Errorf("parse database connection source: %w", errInvalidDatabaseURL) + } + return config, nil +} + +func poolConfig(cfg *PostgresConfig) (*pgxpool.Config, error) { + url, err := resolveURL(cfg.URL) + if err != nil { + return nil, err + } + config, err := parsePoolConfig(url) + if err != nil { + return nil, err + } + if err := applyPoolConfig(config, cfg); err != nil { + return nil, err + } + vectorSchema := cfg.VectorSchema + if vectorSchema == "" { + vectorSchema = consts.DefaultPgvectorSchema + } + if cfg.VectorEnabled && cfg.Schema == "" { + return nil, errors.New("database schema is required when pgvector is enabled") + } + var searchPath string + if cfg.Schema != "" { + searchPath = pgx.Identifier{cfg.Schema}.Sanitize() + if cfg.Schema != "public" && !cfg.VectorEnabled { + searchPath += ", public" + } + config.ConnConfig.RuntimeParams["search_path"] = searchPath + } + + fileBacked := strings.HasPrefix(cfg.URL, fileSourcePrefix) + if fileBacked || usesTLS(config.ConnConfig) { + baseline := config.ConnConfig + config.BeforeConnect = func(_ context.Context, connConfig *pgx.ConnConfig) error { + url, err := resolveURL(cfg.URL) + if err != nil { + return err + } + fresh, err := parsePoolConfig(url) + if err != nil { + return err + } + if !sameConnectionIdentity(baseline, fresh.ConnConfig) { + return errors.New("database connection identity changed; restart required") + } + + refreshed := fresh.ConnConfig.Config.Copy() + if fileBacked { + if cfg.Role == "" && baseline.User != refreshed.User { + return errors.New("database user changed without a stable role; restart required") + } + // A rotation that issues a new user each cycle, keeping the + // previous one able to log in until the cycle after, needs new + // connections to dial as the incoming user while older ones + // finish on the outgoing one. Only the endpoint is fenced above. + connConfig.User = refreshed.User + connConfig.Password = refreshed.Password + } + connConfig.TLSConfig = refreshed.TLSConfig + connConfig.Fallbacks = refreshed.Fallbacks + return nil + } + } + + if cfg.Role != "" || cfg.VectorEnabled || cfg.Schema != "" { + config.AfterConnect = func(ctx context.Context, conn *pgx.Conn) error { + if cfg.Role != "" { + if _, err := conn.Exec(ctx, "SELECT set_config('role', $1, false)", cfg.Role); err != nil { + return fmt.Errorf("assuming PostgreSQL role %q: %w", cfg.Role, err) + } + } + if cfg.Schema != "" { + var currentSchema string + if err := conn.QueryRow(ctx, "SELECT COALESCE(current_schema(), '')").Scan(¤tSchema); err != nil { + return fmt.Errorf("check PostgreSQL schema %q: %w", cfg.Schema, err) + } + if currentSchema != cfg.Schema { + return fmt.Errorf("PostgreSQL schema %q is not accessible (current schema is %q)", cfg.Schema, currentSchema) + } + } + if cfg.VectorEnabled { + if cfg.Schema != "" && vectorSchema != cfg.Schema { + if _, err := conn.Exec(ctx, "SELECT set_config('search_path', $1, false)", pgx.Identifier{vectorSchema}.Sanitize()); err != nil { + return fmt.Errorf("select pgvector schema %q: %w", vectorSchema, err) + } + } + if err := pgvectorpgx.RegisterTypes(ctx, conn); err != nil { + return err + } + if cfg.Schema != "" && vectorSchema != cfg.Schema { + if _, err := conn.Exec(ctx, "SELECT set_config('search_path', $1, false)", searchPath); err != nil { + return fmt.Errorf("restore PostgreSQL search path: %w", err) + } + } + return nil + } + return nil + } + } + return config, nil +} + +func sameConnectionIdentity(a, b *pgx.ConnConfig) bool { + if a.Host != b.Host || a.Port != b.Port || a.Database != b.Database { + return false + } + return slices.EqualFunc(a.Fallbacks, b.Fallbacks, func(a, b *pgconn.FallbackConfig) bool { + return a.Host == b.Host && a.Port == b.Port + }) +} + +func usesTLS(config *pgx.ConnConfig) bool { + if config.TLSConfig != nil { + return true + } + return slices.ContainsFunc(config.Fallbacks, func(fallback *pgconn.FallbackConfig) bool { + return fallback.TLSConfig != nil + }) +} + // retryDBConnection opens and verifies a pool, registering vector types when enabled. // Failed pings retry with exponential backoff until cancellation or the two-minute // timeout; an unsuccessful pool is closed before returning the error. @@ -72,18 +244,10 @@ func retryDBConnection(ctx context.Context, cfg *PostgresConfig) (*pgxpool.Pool, ctx, cancel := context.WithTimeout(ctx, defaultMaxTimeout) defer cancel() - config, err := pgxpool.ParseConfig(cfg.URL) + config, err := poolConfig(cfg) if err != nil { - return nil, fmt.Errorf("failed to parse database URL: %w", err) - } - if err := applyPoolConfig(config, cfg); err != nil { return nil, err } - if cfg.VectorEnabled { - config.AfterConnect = func(ctx context.Context, conn *pgx.Conn) error { - return pgvectorpgx.RegisterTypes(ctx, conn) - } - } pool, err := pgxpool.NewWithConfig(ctx, config) if err != nil { @@ -110,27 +274,3 @@ func retryDBConnection(ctx context.Context, cfg *PostgresConfig) (*pgxpool.Pool, } } } - -// ResolveURL returns url unless urlFile is set, in which case it returns the file's -// trimmed contents. An unreadable or empty file returns an error without falling -// back to url. -func ResolveURL(url, urlFile string) (string, error) { - if urlFile != "" { - return resolveURLFile(urlFile) - } - return url, nil -} - -// resolveURLFile reads a database connection URL from a file and returns the -// trimmed contents. Returns an error if the file cannot be read or is empty. -func resolveURLFile(path string) (string, error) { - content, err := os.ReadFile(path) - if err != nil { - return "", fmt.Errorf("reading URL file: %w", err) - } - url := strings.TrimSpace(string(content)) - if url == "" { - return "", fmt.Errorf("URL file %s is empty or contains only whitespace", path) - } - return url, nil -} diff --git a/go/core/internal/database/connect_test.go b/go/core/internal/database/connect_test.go index 0525301e68..0e662f5d8c 100644 --- a/go/core/internal/database/connect_test.go +++ b/go/core/internal/database/connect_test.go @@ -2,11 +2,13 @@ package database import ( "context" + "net/url" "os" "path/filepath" "testing" "time" + "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -54,53 +56,269 @@ func TestApplyPoolConfig(t *testing.T) { }) } -func TestResolveURLFile(t *testing.T) { +func TestResolveURL(t *testing.T) { + t.Run("literal", func(t *testing.T) { + const url = "postgres://user:password@localhost:5432/database?sslmode=disable" + got, err := ResolveURL(url) + require.NoError(t, err) + assert.Equal(t, url, got) + }) + + t.Run("file", func(t *testing.T) { + path := filepath.Join(t.TempDir(), "connection-string") + writeDatabaseURL(t, path, " postgres://user:password@localhost:5432/database?sslmode=disable\n") + + got, err := ResolveURL("@file:" + path) + require.NoError(t, err) + assert.Equal(t, "postgres://user:password@localhost:5432/database?sslmode=disable", got) + }) + + t.Run("relative file", func(t *testing.T) { + _, err := ResolveURL("@file:connection-string") + require.Error(t, err) + assert.Contains(t, err.Error(), "must be absolute") + }) + + t.Run("missing file", func(t *testing.T) { + path := filepath.Join(t.TempDir(), "missing") + _, err := ResolveURL("@file:" + path) + require.Error(t, err) + assert.Contains(t, err.Error(), path) + }) + + t.Run("empty file", func(t *testing.T) { + path := filepath.Join(t.TempDir(), "connection-string") + writeDatabaseURL(t, path, " \n") + _, err := ResolveURL("@file:" + path) + require.Error(t, err) + assert.Contains(t, err.Error(), "is empty") + }) + + t.Run("malformed URL is redacted", func(t *testing.T) { + _, err := ResolveURL("postgres://user:do-not-disclose@[invalid") + require.Error(t, err) + assert.ErrorIs(t, err, errInvalidDatabaseURL) + assert.NotContains(t, err.Error(), "do-not-disclose") + }) +} + +func TestPoolConfigRefreshesFileCredentials(t *testing.T) { + path := filepath.Join(t.TempDir(), "connection-string") + const firstURL = "postgres://user:password-a@database:5432/app?sslmode=require&application_name=kagent" + writeDatabaseURL(t, path, firstURL) + + config, err := poolConfig(&PostgresConfig{URL: "@file:" + path, Role: "kagent_app"}) + require.NoError(t, err) + require.NotNil(t, config.BeforeConnect) + require.NotNil(t, config.AfterConnect) + assert.Equal(t, "password-a", config.ConnConfig.Password) + + connConfig := config.ConnConfig.Copy() + initialTLS := connConfig.TLSConfig + writeDatabaseURL(t, path, "postgres://user_v2:password-b@database:5432/app?sslmode=require&application_name=changed") + require.NoError(t, config.BeforeConnect(context.Background(), connConfig)) + + assert.Equal(t, "password-b", connConfig.Password) + // The user rotates with the password; only the endpoint is fenced. + assert.Equal(t, "user_v2", connConfig.User) + assert.Equal(t, "user", config.ConnConfig.User, "refresh must not mutate the pinned config") + assert.Equal(t, "kagent", connConfig.RuntimeParams["application_name"]) + assert.NotSame(t, initialTLS, connConfig.TLSConfig) +} + +func TestPoolConfigSetsSchema(t *testing.T) { + config, err := poolConfig(&PostgresConfig{ + URL: "postgres://user:password@database:5432/app?sslmode=disable", + Schema: "kagent", + }) + require.NoError(t, err) + assert.Equal(t, `"kagent", public`, config.ConnConfig.RuntimeParams["search_path"]) +} + +func TestPoolConfigRejectsMissingRuntimeSchema(t *testing.T) { + if testing.Short() { + t.Skip("skip the PostgreSQL test in short mode") + } + const schema = "missing_kagent_schema_for_connect_test" + config, err := poolConfig(&PostgresConfig{URL: sharedConnStr, Schema: schema}) + require.NoError(t, err) + conn, err := pgx.ConnectConfig(t.Context(), config.ConnConfig.Copy()) + require.NoError(t, err) + defer conn.Close(t.Context()) + + err = config.AfterConnect(t.Context(), conn) + require.ErrorContains(t, err, `PostgreSQL schema "`+schema+`" is not accessible`) +} + +func TestPoolConfigRejectsRotatedUserWithoutStableRole(t *testing.T) { + path := filepath.Join(t.TempDir(), "connection-string") + writeDatabaseURL(t, path, "postgres://user:password-a@database:5432/app?sslmode=disable") + config, err := poolConfig(&PostgresConfig{URL: "@file:" + path}) + require.NoError(t, err) + + writeDatabaseURL(t, path, "postgres://user_v2:password-b@database:5432/app?sslmode=disable") + err = config.BeforeConnect(context.Background(), config.ConnConfig.Copy()) + require.Error(t, err) + assert.Contains(t, err.Error(), "without a stable role") +} + +func TestConnectRotatesLoginBehindStableRole(t *testing.T) { + if testing.Short() { + t.Skip("skip the PostgreSQL test in short mode") + } + const ( + role = "kagent_rotation_role" + loginA = "kagent_rotation_login_a" + loginB = "kagent_rotation_login_b" + ) + _, err := sharedDB.Exec(t.Context(), ` + DROP ROLE IF EXISTS kagent_rotation_login_a; + DROP ROLE IF EXISTS kagent_rotation_login_b; + DROP ROLE IF EXISTS kagent_rotation_role; + CREATE ROLE kagent_rotation_role NOLOGIN; + CREATE ROLE kagent_rotation_login_a LOGIN PASSWORD 'rotation-password'; + CREATE ROLE kagent_rotation_login_b LOGIN PASSWORD 'rotation-password'; + GRANT kagent_rotation_role TO kagent_rotation_login_a, kagent_rotation_login_b`) + require.NoError(t, err) + t.Cleanup(func() { + _, _ = sharedDB.Exec(context.Background(), ` + DROP ROLE IF EXISTS kagent_rotation_login_a; + DROP ROLE IF EXISTS kagent_rotation_login_b; + DROP ROLE IF EXISTS kagent_rotation_role`) + }) + + dsn, err := url.Parse(sharedConnStr) + require.NoError(t, err) + dsn.User = url.UserPassword(loginA, "rotation-password") + path := filepath.Join(t.TempDir(), "connection-string") + writeDatabaseURL(t, path, dsn.String()) + pool, err := Connect(t.Context(), &PostgresConfig{URL: "@file:" + path, Role: role}) + require.NoError(t, err) + defer pool.Close() + + var sessionUser, currentUser string + require.NoError(t, pool.QueryRow(t.Context(), `SELECT session_user, current_user`).Scan(&sessionUser, ¤tUser)) + assert.Equal(t, loginA, sessionUser) + assert.Equal(t, role, currentUser) + + dsn.User = url.UserPassword(loginB, "rotation-password") + writeDatabaseURL(t, path, dsn.String()) + pool.Reset() + require.NoError(t, pool.QueryRow(t.Context(), `SELECT session_user, current_user`).Scan(&sessionUser, ¤tUser)) + assert.Equal(t, loginB, sessionUser) + assert.Equal(t, role, currentUser) +} + +func TestPoolConfigRefreshesTLSForLiteralURL(t *testing.T) { + config, err := poolConfig(&PostgresConfig{ + URL: "postgres://user:static-password@database:5432/app?sslmode=require", + }) + require.NoError(t, err) + require.NotNil(t, config.BeforeConnect) + + connConfig := config.ConnConfig.Copy() + initialTLS := connConfig.TLSConfig + connConfig.Password = "unchanged-password" + require.NoError(t, config.BeforeConnect(context.Background(), connConfig)) + + assert.Equal(t, "unchanged-password", connConfig.Password) + assert.NotSame(t, initialTLS, connConfig.TLSConfig) +} + +func TestPoolConfigRejectsRotatedIdentity(t *testing.T) { + path := filepath.Join(t.TempDir(), "connection-string") + const initial = "host=primary,secondary port=5432,5433 user=runtime password=password-a dbname=app sslmode=disable" + tests := []struct { - name string - fileContent string - wantUrl string - wantErr bool + name string + url string }{ - { - name: "reads URL from file", - fileContent: "postgres://testuser:testpass@host:5432/testdb", - wantUrl: "postgres://testuser:testpass@host:5432/testdb", - }, - { - name: "trims whitespace and newlines", - fileContent: " postgres://user:pass@host:5432/db\n", - wantUrl: "postgres://user:pass@host:5432/db", - }, - { - name: "empty file returns error", - fileContent: "", - wantErr: true, - }, - { - name: "whitespace-only file returns error", - fileContent: " \n\t\n ", - wantErr: true, - }, + {name: "host", url: "host=changed,secondary port=5432,5433 user=runtime password=password-b dbname=app sslmode=disable"}, + {name: "port", url: "host=primary,secondary port=6432,5433 user=runtime password=password-b dbname=app sslmode=disable"}, + {name: "database", url: "host=primary,secondary port=5432,5433 user=runtime password=password-b dbname=changed sslmode=disable"}, + {name: "fallback", url: "host=primary,changed port=5432,5433 user=runtime password=password-b dbname=app sslmode=disable"}, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - tmpFile := filepath.Join(t.TempDir(), "db-url") - err := os.WriteFile(tmpFile, []byte(tt.fileContent), 0600) - assert.NoError(t, err) - - url, err := resolveURLFile(tmpFile) - if tt.wantErr { - assert.Error(t, err) - return - } - assert.NoError(t, err) - assert.Equal(t, tt.wantUrl, url) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + writeDatabaseURL(t, path, initial) + config, err := poolConfig(&PostgresConfig{URL: "@file:" + path}) + require.NoError(t, err) + + writeDatabaseURL(t, path, test.url) + err = config.BeforeConnect(context.Background(), config.ConnConfig.Copy()) + require.Error(t, err) + assert.Contains(t, err.Error(), "restart required") + assert.NotContains(t, err.Error(), "password-a") + assert.NotContains(t, err.Error(), "password-b") }) } +} + +func TestPoolConfigFailsSafelyWhenReplacementIsInvalid(t *testing.T) { + path := filepath.Join(t.TempDir(), "connection-string") + writeDatabaseURL(t, path, "postgres://user:password-a@database:5432/app?sslmode=disable") + config, err := poolConfig(&PostgresConfig{URL: "@file:" + path}) + require.NoError(t, err) + + t.Run("malformed", func(t *testing.T) { + writeDatabaseURL(t, path, "postgres://user:replacement-secret@[invalid") + err := config.BeforeConnect(context.Background(), config.ConnConfig.Copy()) + require.Error(t, err) + assert.ErrorIs(t, err, errInvalidDatabaseURL) + assert.NotContains(t, err.Error(), "replacement-secret") + }) - t.Run("missing file returns error", func(t *testing.T) { - _, err := resolveURLFile("/nonexistent/path/db-url") - assert.Error(t, err) + t.Run("empty", func(t *testing.T) { + writeDatabaseURL(t, path, " \n") + err := config.BeforeConnect(context.Background(), config.ConnConfig.Copy()) + require.Error(t, err) + assert.Contains(t, err.Error(), "is empty") }) + + t.Run("unreadable", func(t *testing.T) { + require.NoError(t, os.Remove(path)) + err := config.BeforeConnect(context.Background(), config.ConnConfig.Copy()) + require.Error(t, err) + assert.Contains(t, err.Error(), "read database connection source") + }) +} + +func TestPoolConfigPreservesHooksAndLimits(t *testing.T) { + maxConns := int32(8) + minConns := int32(1) + idleTime := time.Minute + lifetime := 10 * time.Minute + config, err := poolConfig(&PostgresConfig{ + URL: "postgres://user:password@database:5432/app?sslmode=disable", + VectorEnabled: true, + Schema: "public", + VectorSchema: "public", + MaxConns: &maxConns, + MinConns: &minConns, + MaxConnIdleTime: &idleTime, + MaxConnLifetime: &lifetime, + }) + require.NoError(t, err) + + assert.Nil(t, config.BeforeConnect) + assert.NotNil(t, config.AfterConnect) + assert.Equal(t, maxConns, config.MaxConns) + assert.Equal(t, minConns, config.MinConns) + assert.Equal(t, idleTime, config.MaxConnIdleTime) + assert.Equal(t, lifetime, config.MaxConnLifetime) +} + +func TestPoolConfigRequiresTableSchemaForVectors(t *testing.T) { + _, err := poolConfig(&PostgresConfig{ + URL: "postgres://user:password@database:5432/app?sslmode=disable", + VectorEnabled: true, + VectorSchema: "public", + }) + require.ErrorContains(t, err, "database schema is required") +} + +func writeDatabaseURL(t *testing.T, path, url string) { + t.Helper() + require.NoError(t, os.WriteFile(path, []byte(url), 0o600)) } diff --git a/go/core/internal/database/memory.go b/go/core/internal/database/memory.go index 2a65ee4e62..126fb237e6 100644 --- a/go/core/internal/database/memory.go +++ b/go/core/internal/database/memory.go @@ -56,17 +56,17 @@ func (c *Client) StoreAgentMemories(ctx context.Context, memories ...*Memory) er // best-effort and cannot fail a successful search. func (c *Client) SearchAgentMemory(ctx context.Context, agentName, userID string, embedding pgvector.Vector, limit int) ([]AgentMemorySearchResult, error) { normalized := strings.ReplaceAll(agentName, "-", "_") - results, err := queryMany(ctx, c.db, ` + results, err := queryMany(ctx, c.db, fmt.Sprintf(` SELECT id, COALESCE(agent_name, '') AS agent_name, COALESCE(user_id, '') AS user_id, COALESCE(content, '') AS content, embedding, COALESCE(metadata, '') AS metadata, COALESCE(created_at, '0001-01-01 00:00:00+00'::timestamptz) AS created_at, expires_at, COALESCE(access_count, 0) AS access_count, - COALESCE(1 - (embedding <=> $1), 0) AS score + COALESCE(1 - (embedding %s $1), 0) AS score FROM memory WHERE (agent_name = $2 OR agent_name = $3) AND user_id = $4 - ORDER BY embedding <=> $1 ASC + ORDER BY embedding %s $1 ASC LIMIT $5 - `, pgx.RowToStructByName[AgentMemorySearchResult], embedding, &agentName, &normalized, &userID, int32(limit)) + `, c.vectorCosineOperator, c.vectorCosineOperator), pgx.RowToStructByName[AgentMemorySearchResult], embedding, &agentName, &normalized, &userID, int32(limit)) if err != nil { return nil, fmt.Errorf("failed to search agent memory: %w", err) } diff --git a/go/core/internal/database/sql_test.go b/go/core/internal/database/sql_test.go index d90ad5f940..d91f963cff 100644 --- a/go/core/internal/database/sql_test.go +++ b/go/core/internal/database/sql_test.go @@ -51,10 +51,30 @@ func TestInlineSQLPrepares(t *testing.T) { return true } position := positions.Position(call.Pos()) - literal, ok := call.Args[index].(*ast.BasicLit) - require.True(t, ok, "%s: keep SQL literal so schema validation covers it", position) - sql, err := strconv.Unquote(literal.Value) - require.NoError(t, err) + var sql string + switch value := call.Args[index].(type) { + case *ast.BasicLit: + sql, err = strconv.Unquote(value.Value) + require.NoError(t, err) + case *ast.CallExpr: + formatter, ok := value.Fun.(*ast.SelectorExpr) + require.True(t, ok, "%s: keep SQL literal so schema validation covers it", position) + require.Equal(t, "Sprintf", formatter.Sel.Name, "%s: unsupported SQL expression", position) + require.Len(t, value.Args, 3, "%s: expected two vector operators", position) + literal, ok := value.Args[0].(*ast.BasicLit) + require.True(t, ok, "%s: keep SQL template literal", position) + template, unquoteErr := strconv.Unquote(literal.Value) + require.NoError(t, unquoteErr) + require.Equal(t, 2, strings.Count(template, "%s"), "%s: expected two vector operators", position) + for _, arg := range value.Args[1:] { + operator, ok := arg.(*ast.SelectorExpr) + require.True(t, ok, "%s: expected configured vector operator", position) + require.Equal(t, "vectorCosineOperator", operator.Sel.Name, "%s: expected configured vector operator", position) + } + sql = fmt.Sprintf(template, `OPERATOR("public".<=>)`, `OPERATOR("public".<=>)`) + default: + require.FailNow(t, fmt.Sprintf("%s: keep SQL literal so schema validation covers it", position)) + } if !seen[sql] { seen[sql] = true t.Run(fmt.Sprintf("%s:%d", path, position.Line), func(t *testing.T) { diff --git a/go/core/internal/database/testhelpers_test.go b/go/core/internal/database/testhelpers_test.go index cd2c9ed1fd..ce97c31a85 100644 --- a/go/core/internal/database/testhelpers_test.go +++ b/go/core/internal/database/testhelpers_test.go @@ -34,7 +34,7 @@ func TestMain(m *testing.M) { os.Exit(1) } - db, err := Connect(context.Background(), &PostgresConfig{URL: connStr, VectorEnabled: true}) + db, err := Connect(context.Background(), &PostgresConfig{URL: connStr, VectorEnabled: true, Schema: "public", VectorSchema: "public"}) if err != nil { fmt.Fprintf(os.Stderr, "failed to connect to test database: %v\n", err) os.Exit(1) diff --git a/go/core/internal/dbtest/dbtest.go b/go/core/internal/dbtest/dbtest.go index 0a3e7d7236..1ad8df9ea3 100644 --- a/go/core/internal/dbtest/dbtest.go +++ b/go/core/internal/dbtest/dbtest.go @@ -3,6 +3,7 @@ package dbtest import ( "context" + "database/sql" "fmt" "testing" "time" @@ -60,11 +61,21 @@ func StartT(ctx context.Context, t *testing.T) string { return connStr } -// Migrate runs the embedded migrations against connStr and returns any error. -// If vectorEnabled is true the vector pass is also applied. +// Migrate installs pgvector for the test database when enabled, then runs the +// embedded migrations against connStr. // Use MigrateT in tests that have a *testing.T; use Migrate in TestMain where no T is available. func Migrate(connStr string, vectorEnabled bool) error { - return migrations.RunUp(context.Background(), connStr, migrations.BuiltinSources(vectorEnabled)) + if vectorEnabled { + db, err := sql.Open("pgx", connStr) + if err != nil { + return err + } + defer db.Close() + if _, err := db.Exec("CREATE EXTENSION IF NOT EXISTS vector WITH SCHEMA public"); err != nil { + return err + } + } + return migrations.RunUp(context.Background(), connStr, migrations.BuiltinSourcesInSchema(vectorEnabled, "public", "public")) } // MigrateT runs the embedded migrations against connStr and calls t.Fatal on error. diff --git a/go/core/pkg/app/app.go b/go/core/pkg/app/app.go index 6983f88c74..f3cc15f083 100644 --- a/go/core/pkg/app/app.go +++ b/go/core/pkg/app/app.go @@ -172,27 +172,29 @@ func Run(ctx context.Context, opts Options) error { } }() - dbURL, err := database.ResolveURL(env("POSTGRES_DATABASE_URL", "postgres://postgres:kagent@kagent-postgresql.kagent.svc.cluster.local:5432/postgres"), os.Getenv("POSTGRES_DATABASE_URL_FILE")) + dbSource := env("POSTGRES_DATABASE_URL", "postgres://postgres:kagent@kagent-postgresql.kagent.svc.cluster.local:5432/postgres") + dbURL, err := database.ResolveURL(dbSource) if err != nil { - return err + return fmt.Errorf("resolve database connection: %w", err) } vectorEnabled := kagentenv.DatabaseVectorEnabled.Get() + dbRole := kagentenv.DatabaseRole.Get() // Appended, not merged: the built-in tracks must reach their final version // before a library consumer's tables, which may reference them. - sources := append(migrations.BuiltinSources(vectorEnabled), opts.ExtraMigrations...) + sources := append(migrations.BuiltinSourcesInSchema(vectorEnabled, kagentenv.DatabaseSchema.Get(), kagentenv.DatabaseVectorSchema.Get()), opts.ExtraMigrations...) if kagentenv.SkipMigrations.Get() { - if err := migrations.VerifyMigrated(ctx, dbURL, sources); err != nil { + if err := migrations.VerifyMigratedAsRole(ctx, dbURL, dbRole, sources); err != nil { return fmt.Errorf("verify database migrations: %w", err) } - } else if err := migrations.RunUp(ctx, dbURL, sources); err != nil { + } else if err := migrations.RunUpAsRole(ctx, dbURL, dbRole, sources); err != nil { return fmt.Errorf("run database migrations: %w", err) } - db, err := database.Connect(ctx, &database.PostgresConfig{URL: dbURL, VectorEnabled: vectorEnabled}) + db, err := database.Connect(ctx, postgresConfigFromEnv(dbSource, vectorEnabled)) if err != nil { return err } defer db.Close() - store := database.NewClient(db) + store := database.NewClient(db, kagentenv.DatabaseVectorSchema.Get()) kubeConfig, err := clientcmd.NewNonInteractiveDeferredLoadingClientConfig( clientcmd.NewDefaultClientConfigLoadingRules(), &clientcmd.ConfigOverrides{}, @@ -386,6 +388,31 @@ func envBool(name string) bool { return value } +func postgresConfigFromEnv(source string, vectorEnabled bool) *database.PostgresConfig { + config := &database.PostgresConfig{ + URL: source, + Role: kagentenv.DatabaseRole.Get(), + Schema: kagentenv.DatabaseSchema.Get(), + VectorSchema: kagentenv.DatabaseVectorSchema.Get(), + VectorEnabled: vectorEnabled, + } + if value := kagentenv.DatabaseMaxConns.Get(); value > 0 { + maxConns := int32(value) + config.MaxConns = &maxConns + } + if value := kagentenv.DatabaseMinConns.Get(); value >= 0 { + minConns := int32(value) + config.MinConns = &minConns + } + if value := kagentenv.DatabaseMaxConnIdleTime.Get(); value > 0 { + config.MaxConnIdleTime = &value + } + if value := kagentenv.DatabaseMaxConnLifetime.Get(); value > 0 { + config.MaxConnLifetime = &value + } + return config +} + // metricsBindAddress resolves METRICS_BIND_ADDRESS. controller-runtime reads an // empty address as "unset" and falls back to :8080, so an empty value would // serve metrics on a port nobody asked for. "0" disables the metrics server. diff --git a/go/core/pkg/app/app_test.go b/go/core/pkg/app/app_test.go index ef5fdaa335..6b27661111 100644 --- a/go/core/pkg/app/app_test.go +++ b/go/core/pkg/app/app_test.go @@ -7,6 +7,7 @@ import ( "net/url" "reflect" "testing" + "time" apiauthorization "github.com/kagent-dev/kagent/go/api/authorization" "github.com/kagent-dev/kagent/go/core/internal/grpcserver" @@ -152,6 +153,38 @@ func TestNamespaceCache(t *testing.T) { } } +func TestPostgresConfigFromEnv(t *testing.T) { + t.Setenv("DB_MAX_CONNS", "8") + t.Setenv("DB_MIN_CONNS", "1") + t.Setenv("DB_MAX_CONN_IDLE_TIME", "1m") + t.Setenv("DB_MAX_CONN_LIFETIME", "10m") + t.Setenv("POSTGRES_DATABASE_ROLE", "kagent_app") + t.Setenv("POSTGRES_DATABASE_SCHEMA", "kagent_test") + + config := postgresConfigFromEnv("@file:/database/connection-string", true) + if config.URL != "@file:/database/connection-string" || !config.VectorEnabled { + t.Fatalf("postgres config lost connection source or vector setting: %#v", config) + } + if config.Role != "kagent_app" { + t.Fatalf("Role = %q, want kagent_app", config.Role) + } + if config.Schema != "kagent_test" { + t.Fatalf("Schema = %q, want kagent_test", config.Schema) + } + if config.MaxConns == nil || *config.MaxConns != 8 { + t.Fatalf("MaxConns = %v, want 8", config.MaxConns) + } + if config.MinConns == nil || *config.MinConns != 1 { + t.Fatalf("MinConns = %v, want 1", config.MinConns) + } + if config.MaxConnIdleTime == nil || *config.MaxConnIdleTime != time.Minute { + t.Fatalf("MaxConnIdleTime = %v, want 1m", config.MaxConnIdleTime) + } + if config.MaxConnLifetime == nil || *config.MaxConnLifetime != 10*time.Minute { + t.Fatalf("MaxConnLifetime = %v, want 10m", config.MaxConnLifetime) + } +} + // The built-in tracks must reach their final version before a library consumer's, // which may reference them, so order is the contract here -- not membership. func TestExtraMigrationsAppendAfterBuiltins(t *testing.T) { diff --git a/go/core/pkg/consts/postgres.go b/go/core/pkg/consts/postgres.go new file mode 100644 index 0000000000..16216572ab --- /dev/null +++ b/go/core/pkg/consts/postgres.go @@ -0,0 +1,6 @@ +package consts + +const ( + DefaultPostgresTableSchema = "kagent" + DefaultPgvectorSchema = "extensions" +) diff --git a/go/core/pkg/env/kagent.go b/go/core/pkg/env/kagent.go index 25008812df..d1a78383ba 100644 --- a/go/core/pkg/env/kagent.go +++ b/go/core/pkg/env/kagent.go @@ -1,5 +1,7 @@ package env +import "github.com/kagent-dev/kagent/go/core/pkg/consts" + // Core kagent environment variables used by the controller and agent runtime. var ( LeaderElect = RegisterBoolVar( @@ -137,4 +139,53 @@ var ( "Verify required database migrations at startup without applying them.", ComponentDatabase, ) + + DatabaseRole = RegisterStringVar( + "POSTGRES_DATABASE_ROLE", + "", + "Stable PostgreSQL role assumed after authentication. Required for rotation to a different login user.", + ComponentDatabase, + ) + + DatabaseSchema = RegisterStringVar( + "POSTGRES_DATABASE_SCHEMA", + consts.DefaultPostgresTableSchema, + "PostgreSQL schema for Kagent tables.", + ComponentDatabase, + ) + + DatabaseVectorSchema = RegisterStringVar( + "POSTGRES_VECTOR_SCHEMA", + consts.DefaultPgvectorSchema, + "Schema where the shared pgvector extension is installed.", + ComponentDatabase, + ) + + DatabaseMaxConns = RegisterIntVar( + "DB_MAX_CONNS", + 0, + "Maximum number of PostgreSQL pool connections. Zero keeps the pgx default.", + ComponentDatabase, + ) + + DatabaseMinConns = RegisterIntVar( + "DB_MIN_CONNS", + -1, + "Minimum number of PostgreSQL pool connections. Negative keeps the pgx default.", + ComponentDatabase, + ) + + DatabaseMaxConnIdleTime = RegisterDurationVar( + "DB_MAX_CONN_IDLE_TIME", + 0, + "Maximum idle time for a PostgreSQL pool connection. Zero keeps the pgx default.", + ComponentDatabase, + ) + + DatabaseMaxConnLifetime = RegisterDurationVar( + "DB_MAX_CONN_LIFETIME", + 0, + "Maximum lifetime of a PostgreSQL pool connection. This bounds credential rotation time.", + ComponentDatabase, + ) ) diff --git a/go/core/pkg/migrations/identity/bootstrap.sql b/go/core/pkg/migrations/identity/bootstrap.sql new file mode 100644 index 0000000000..d02a79c6fb --- /dev/null +++ b/go/core/pkg/migrations/identity/bootstrap.sql @@ -0,0 +1,75 @@ +-- PostgreSQL identity setup for Kagent. This is separate from table migrations. +-- Run as an administrator inside a transaction after setting these transaction-local +-- settings: kagent.bootstrap_username, kagent.bootstrap_password, +-- kagent.bootstrap_schema, kagent.bootstrap_vector_enabled, and optionally +-- kagent.bootstrap_vector_schema (default extensions). Optionally set +-- kagent.bootstrap_owner_role (default kagent_owner) for a manually +-- provisioned install. +-- The bundled bootstrap supplies its fixed development credentials and schema. +-- Operators may supply their own values when running this file directly. + +DO $bootstrap$ +DECLARE + app_user text := current_setting('kagent.bootstrap_username'); + app_password text := current_setting('kagent.bootstrap_password'); + schema_name text := current_setting('kagent.bootstrap_schema'); + owner_role text := COALESCE(NULLIF(current_setting('kagent.bootstrap_owner_role', true), ''), 'kagent_owner'); + vector_enabled boolean := current_setting('kagent.bootstrap_vector_enabled')::boolean; + vector_schema text := COALESCE(NULLIF(current_setting('kagent.bootstrap_vector_schema', true), ''), 'extensions'); + installed_vector_schema text; + role_attrs record; + schema_owner text; +BEGIN + IF app_user = '' OR app_password = '' OR schema_name = '' THEN + RAISE EXCEPTION 'Kagent bootstrap username, password, and schema must not be empty'; + END IF; + IF owner_role = app_user THEN + RAISE EXCEPTION 'Kagent owner role and login username must differ'; + END IF; + PERFORM pg_advisory_xact_lock(hashtextextended('kagent:bootstrap:' || schema_name, 0)); + + SELECT rolcanlogin, rolsuper, rolcreatedb, rolcreaterole, rolreplication, rolinherit + INTO role_attrs FROM pg_roles WHERE rolname = owner_role; + IF NOT FOUND THEN + EXECUTE format('CREATE ROLE %I NOLOGIN NOSUPERUSER NOCREATEDB NOCREATEROLE NOREPLICATION', owner_role); + ELSIF role_attrs.rolcanlogin OR role_attrs.rolsuper OR role_attrs.rolcreatedb + OR role_attrs.rolcreaterole OR role_attrs.rolreplication THEN + RAISE EXCEPTION 'managed PostgreSQL role "%" conflicts with the required attributes', owner_role; + END IF; + + SELECT rolcanlogin, rolsuper, rolcreatedb, rolcreaterole, rolreplication, rolinherit + INTO role_attrs FROM pg_roles WHERE rolname = app_user; + IF NOT FOUND THEN + EXECUTE format('CREATE ROLE %I LOGIN NOINHERIT NOSUPERUSER NOCREATEDB NOCREATEROLE NOREPLICATION PASSWORD %L', app_user, app_password); + ELSIF NOT role_attrs.rolcanlogin OR role_attrs.rolsuper OR role_attrs.rolcreatedb + OR role_attrs.rolcreaterole OR role_attrs.rolreplication OR role_attrs.rolinherit THEN + RAISE EXCEPTION 'managed PostgreSQL role "%" conflicts with the required attributes', app_user; + END IF; + + EXECUTE format('GRANT %I TO %I', owner_role, app_user); + SELECT pg_get_userbyid(nspowner) INTO schema_owner FROM pg_namespace WHERE nspname = schema_name; + IF NOT FOUND THEN + EXECUTE format('CREATE SCHEMA %I AUTHORIZATION %I', schema_name, owner_role); + ELSIF schema_name = 'public' THEN + EXECUTE format('GRANT USAGE, CREATE ON SCHEMA public TO %I', owner_role); + ELSIF schema_owner <> owner_role THEN + RAISE EXCEPTION 'PostgreSQL schema "%" is owned by "%", not "%"', schema_name, schema_owner, owner_role; + END IF; + + REVOKE CREATE ON SCHEMA public FROM PUBLIC; + EXECUTE format('REVOKE ALL ON SCHEMA %I FROM PUBLIC', schema_name); + IF vector_enabled THEN + SELECT n.nspname INTO installed_vector_schema + FROM pg_extension e JOIN pg_namespace n ON n.oid = e.extnamespace + WHERE e.extname = 'vector'; + IF FOUND AND installed_vector_schema <> vector_schema THEN + RAISE EXCEPTION 'pgvector is installed in schema "%", expected "%"', installed_vector_schema, vector_schema; + END IF; + IF NOT EXISTS (SELECT 1 FROM pg_namespace WHERE nspname = vector_schema) THEN + EXECUTE format('CREATE SCHEMA %I', vector_schema); + END IF; + EXECUTE format('GRANT USAGE ON SCHEMA %I TO %I', vector_schema, owner_role); + EXECUTE format('CREATE EXTENSION IF NOT EXISTS vector WITH SCHEMA %I', vector_schema); + END IF; +END +$bootstrap$; diff --git a/go/core/pkg/migrations/migrations.go b/go/core/pkg/migrations/migrations.go index 50c3f22d5e..904a62d9c9 100644 --- a/go/core/pkg/migrations/migrations.go +++ b/go/core/pkg/migrations/migrations.go @@ -4,5 +4,5 @@ package migrations import "embed" -//go:embed core vector +//go:embed core vector identity var FS embed.FS diff --git a/go/core/pkg/migrations/runner.go b/go/core/pkg/migrations/runner.go index 50b85995cf..70e97358c1 100644 --- a/go/core/pkg/migrations/runner.go +++ b/go/core/pkg/migrations/runner.go @@ -14,7 +14,9 @@ import ( "strconv" "strings" - _ "github.com/jackc/pgx/v5/stdlib" + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/stdlib" + "github.com/kagent-dev/kagent/go/core/pkg/consts" "github.com/pressly/goose/v3" "github.com/pressly/goose/v3/lock" ) @@ -34,27 +36,44 @@ var ( type Source struct { Name string Schema string + VectorSchema string TrackingTable string FS fs.FS Dir string PreCheck func(url string) error } -// BuiltinSources returns the built-in migration sources. +// BuiltinSources returns the built-in migration sources in the default schemas. func BuiltinSources(vectorEnabled bool) []Source { + return BuiltinSourcesInSchema(vectorEnabled, consts.DefaultPostgresTableSchema, consts.DefaultPgvectorSchema) +} + +// BuiltinSourcesInSchema returns the built-in sources with their table and +// pgvector schemas selected independently. Empty values use the defaults. +func BuiltinSourcesInSchema(vectorEnabled bool, schema, vectorSchema string) []Source { + if schema == "" { + schema = consts.DefaultPostgresTableSchema + } + if vectorSchema == "" { + vectorSchema = consts.DefaultPgvectorSchema + } sources := []Source{{ Name: "core", + Schema: schema, TrackingTable: coreTrackingTable, FS: FS, Dir: "core", }} if vectorEnabled { + sources[0].VectorSchema = vectorSchema sources = append(sources, Source{ Name: "vector", + Schema: schema, + VectorSchema: vectorSchema, TrackingTable: vectorTrackingTable, FS: FS, Dir: "vector", - PreCheck: checkPgvector, + PreCheck: pgvectorPreCheck(vectorSchema), }) } return sources @@ -62,13 +81,19 @@ func BuiltinSources(vectorEnabled bool) []Source { // RunUp applies all pending migrations in source order. func RunUp(ctx context.Context, url string, sources []Source) error { + return RunUpAsRole(ctx, url, "", sources) +} + +// RunUpAsRole applies all pending migrations after assuming role on every +// database connection. The authenticated login only needs membership in role. +func RunUpAsRole(ctx context.Context, url, role string, sources []Source) error { if len(sources) == 0 { return nil } if err := validateSources(sources); err != nil { return err } - if err := checkResolvedSchemaCollisions(ctx, url, sources); err != nil { + if err := checkResolvedSchemaCollisions(ctx, url, role, sources); err != nil { return err } @@ -88,7 +113,7 @@ func RunUp(ctx context.Context, url string, sources []Source) error { if err := ctx.Err(); err != nil { return fmt.Errorf("cancel before %s migrations: %w", src.Name, err) } - err := WithProvider(ctx, url, src, func(provider *goose.Provider) error { + err := withProvider(ctx, url, role, src, func(provider *goose.Provider) error { _, err := provider.Up(ctx) return err }) @@ -101,17 +126,30 @@ func RunUp(ctx context.Context, url string, sources []Source) error { // VerifyMigrated checks migration state without database writes. func VerifyMigrated(ctx context.Context, url string, sources []Source) error { + return VerifyMigratedAsRole(ctx, url, "", sources) +} + +// VerifyMigratedAsRole checks migration state after assuming role on every +// database connection. +func VerifyMigratedAsRole(ctx context.Context, url, role string, sources []Source) error { if len(sources) == 0 { return nil } if err := validateSources(sources); err != nil { return err } - if err := checkResolvedSchemaCollisions(ctx, url, sources); err != nil { + if err := checkResolvedSchemaCollisions(ctx, url, role, sources); err != nil { return err } + for _, src := range sources { + if src.PreCheck != nil { + if err := src.PreCheck(url); err != nil { + return fmt.Errorf("%s precheck: %w", src.Name, err) + } + } + } - db, err := sql.Open("pgx", url) + db, err := openDB(url, role) if err != nil { return fmt.Errorf("open database: %w", err) } @@ -173,19 +211,29 @@ func VerifyMigrated(ctx context.Context, url string, sources []Source) error { // WithProvider runs fn while one source lock is held. func WithProvider(ctx context.Context, url string, src Source, fn func(*goose.Provider) error) (retErr error) { + return withProvider(ctx, url, "", src, fn) +} + +// WithProviderAsRole runs fn while one source lock is held and every database +// connection has assumed role. +func WithProviderAsRole(ctx context.Context, url, role string, src Source, fn func(*goose.Provider) error) error { + return withProvider(ctx, url, role, src, fn) +} + +func withProvider(ctx context.Context, url, role string, src Source, fn func(*goose.Provider) error) (retErr error) { if err := validateSources([]Source{src}); err != nil { return err } connURL := url - if src.Schema != "" { + if src.Schema != "" || src.VectorSchema != "" { var err error - connURL, err = withSearchPath(url, src.Schema) + connURL, err = withSearchPath(url, src.Schema, src.VectorSchema) if err != nil { return fmt.Errorf("set search path for %s: %w", src.Name, err) } } - db, err := sql.Open("pgx", connURL) + db, err := openDB(connURL, role) if err != nil { return fmt.Errorf("open database for %s: %w", src.Name, err) } @@ -194,8 +242,14 @@ func WithProvider(ctx context.Context, url string, src Source, fn func(*goose.Pr }() if src.Schema != "" { - if _, err := db.ExecContext(ctx, "CREATE SCHEMA IF NOT EXISTS "+quoteIdentifier(src.Schema)); err != nil { - return fmt.Errorf("create schema %s: %w", src.Schema, err) + var exists bool + if err := db.QueryRowContext(ctx, "SELECT EXISTS (SELECT 1 FROM pg_namespace WHERE nspname = $1)", src.Schema).Scan(&exists); err != nil { + return fmt.Errorf("check schema %s: %w", src.Schema, err) + } + if !exists { + if _, err := db.ExecContext(ctx, "CREATE SCHEMA IF NOT EXISTS "+quoteIdentifier(src.Schema)); err != nil { + return fmt.Errorf("create schema %s: %w", src.Schema, err) + } } } @@ -204,6 +258,9 @@ func WithProvider(ctx context.Context, url string, src Source, fn func(*goose.Pr if err := db.QueryRowContext(ctx, "SELECT current_database(), current_schema()").Scan(&databaseName, &schemaName); err != nil { return fmt.Errorf("resolve database identity: %w", err) } + if src.Schema != "" && schemaName.String != src.Schema { + return fmt.Errorf("migration schema %q is not accessible (current schema is %q)", src.Schema, schemaName.String) + } if !schemaName.Valid { return errors.New("the connection has no current schema") } @@ -283,6 +340,14 @@ func validateSources(sources []Source) error { return fmt.Errorf("source %s: %w", src.Name, err) } } + if src.VectorSchema != "" { + if err := validateIdentifier("pgvector schema", src.VectorSchema); err != nil { + return fmt.Errorf("source %s: %w", src.Name, err) + } + if src.Schema == "" { + return fmt.Errorf("source %s needs a table schema when pgvector is enabled", src.Name) + } + } if err := validateIdentifier("tracking table", src.TrackingTable); err != nil { return fmt.Errorf("source %s: %w", src.Name, err) } @@ -367,7 +432,7 @@ func gooseAnnotation(line string) string { return strings.ToLower(strings.TrimSpace(command)) } -func checkResolvedSchemaCollisions(ctx context.Context, url string, sources []Source) error { +func checkResolvedSchemaCollisions(ctx context.Context, url, role string, sources []Source) error { var hasDefault, hasExplicit bool for _, src := range sources { if src.Schema == "" { @@ -380,7 +445,7 @@ func checkResolvedSchemaCollisions(ctx context.Context, url string, sources []So return nil } - db, err := sql.Open("pgx", url) + db, err := openDB(url, role) if err != nil { return fmt.Errorf("open database to resolve schema: %w", err) } @@ -408,23 +473,45 @@ func checkResolvedSchemaCollisions(ctx context.Context, url string, sources []So return nil } -func checkPgvector(url string) error { - db, err := sql.Open("pgx", url) - if err != nil { - return fmt.Errorf("open database: %w", err) +func openDB(url, role string) (*sql.DB, error) { + if role == "" { + return sql.Open("pgx", url) } - defer db.Close() - var available bool - if err := db.QueryRow("SELECT EXISTS(SELECT 1 FROM pg_available_extensions WHERE name = 'vector')").Scan(&available); err != nil { - return fmt.Errorf("check pgvector: %w", err) + config, err := pgx.ParseConfig(url) + if err != nil { + return nil, errors.New("invalid PostgreSQL connection string") } - if !available { - return errors.New("pgvector is unavailable. Install it or disable database vectors") + return stdlib.OpenDB(*config, stdlib.OptionAfterConnect(func(ctx context.Context, conn *pgx.Conn) error { + if _, err := conn.Exec(ctx, "SELECT set_config('role', $1, false)", role); err != nil { + return fmt.Errorf("assuming PostgreSQL role %q: %w", role, err) + } + return nil + })), nil +} + +func pgvectorPreCheck(expectedSchema string) func(string) error { + return func(url string) error { + db, err := sql.Open("pgx", url) + if err != nil { + return fmt.Errorf("open database: %w", err) + } + defer db.Close() + var schema string + err = db.QueryRow(`SELECT n.nspname FROM pg_extension e JOIN pg_namespace n ON n.oid = e.extnamespace WHERE e.extname = 'vector'`).Scan(&schema) + if errors.Is(err, sql.ErrNoRows) { + return fmt.Errorf("pgvector is not installed in schema %q", expectedSchema) + } + if err != nil { + return fmt.Errorf("check pgvector schema: %w", err) + } + if schema != expectedSchema { + return fmt.Errorf("pgvector is installed in schema %q, expected %q", schema, expectedSchema) + } + return nil } - return nil } -func withSearchPath(dbURL, schema string) (string, error) { +func withSearchPath(dbURL, schema, vectorSchema string) (string, error) { u, err := nurl.Parse(dbURL) if err != nil { return "", fmt.Errorf("parse database URL: %w", err) @@ -433,7 +520,12 @@ func withSearchPath(dbURL, schema string) (string, error) { return "", fmt.Errorf("database URL has unsupported scheme %q", u.Scheme) } query := u.Query() - query.Set("search_path", schema) + if schema != "" { + query.Set("search_path", pgx.Identifier{schema}.Sanitize()) + } + if vectorSchema != "" { + query.Set("kagent.vector_schema", vectorSchema) + } u.RawQuery = query.Encode() return u.String(), nil } diff --git a/go/core/pkg/migrations/runner_test.go b/go/core/pkg/migrations/runner_test.go index 5a04c0f202..38a0fe7fe6 100644 --- a/go/core/pkg/migrations/runner_test.go +++ b/go/core/pkg/migrations/runner_test.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "errors" + "net/url" "slices" "strings" "testing" @@ -70,6 +71,7 @@ func startTestDB(t *testing.T) string { if err != nil { t.Fatalf("get PostgreSQL URL: %v", err) } + execSQL(t, dsn, "CREATE EXTENSION vector WITH SCHEMA public") return dsn } @@ -192,9 +194,97 @@ func TestRunUpAndDown(t *testing.T) { } } +func TestRunUpAsStableRole(t *testing.T) { + dsn := startTestDB(t) + execSQL(t, dsn, ` + CREATE ROLE kagent_app NOLOGIN; + CREATE ROLE kagent_login LOGIN PASSWORD 'rotating-password'; + GRANT kagent_app TO kagent_login; + GRANT USAGE, CREATE ON SCHEMA public TO kagent_app`) + t.Cleanup(func() { + execSQL(t, dsn, ` + DROP TABLE IF EXISTS migration_test, test_schema_migrations; + REVOKE ALL ON SCHEMA public FROM kagent_app; + DROP ROLE IF EXISTS kagent_login; + DROP ROLE IF EXISTS kagent_app`) + }) + + loginURL, err := url.Parse(dsn) + if err != nil { + t.Fatal(err) + } + loginURL.User = url.UserPassword("kagent_login", "rotating-password") + if err := RunUpAsRole(t.Context(), loginURL.String(), "kagent_app", []Source{testSource(twoMigrationFS)}); err != nil { + t.Fatal(err) + } + if err := VerifyMigratedAsRole(t.Context(), loginURL.String(), "kagent_app", []Source{testSource(twoMigrationFS)}); err != nil { + t.Fatal(err) + } + + db, err := sql.Open("pgx", dsn) + if err != nil { + t.Fatal(err) + } + defer db.Close() + var owner string + if err := db.QueryRowContext(t.Context(), `SELECT pg_get_userbyid(relowner) FROM pg_class WHERE oid = 'migration_test'::regclass`).Scan(&owner); err != nil { + t.Fatal(err) + } + if owner != "kagent_app" { + t.Fatalf("migration table owner = %q, want kagent_app", owner) + } +} + +func TestCustomSchemaUsesConfiguredVectorSchema(t *testing.T) { + dsn := startTestDB(t) + execSQL(t, dsn, `DROP EXTENSION vector; CREATE SCHEMA extensions; CREATE EXTENSION vector WITH SCHEMA extensions`) + sources := BuiltinSourcesInSchema(true, "tenant_one", "extensions") + if err := RunUp(t.Context(), dsn, sources); err != nil { + t.Fatal(err) + } + if err := VerifyMigrated(t.Context(), dsn, sources); err != nil { + t.Fatal(err) + } + for _, table := range []string{"memory", coreTrackingTable, vectorTrackingTable} { + if !testTableExists(t, dsn, "tenant_one."+table) { + t.Fatalf("%s was not created in tenant_one", table) + } + if testTableExists(t, dsn, "public."+table) || testTableExists(t, dsn, "extensions."+table) { + t.Fatalf("%s was created outside tenant_one", table) + } + } +} + +func TestCustomSchemaMustBeAccessible(t *testing.T) { + dsn := startTestDB(t) + execSQL(t, dsn, `CREATE SCHEMA locked; REVOKE ALL ON SCHEMA locked FROM PUBLIC; CREATE ROLE blocked NOLOGIN`) + source := testSource(twoMigrationFS) + source.Schema = "locked" + if err := RunUpAsRole(t.Context(), dsn, "blocked", []Source{source}); err == nil || !strings.Contains(err.Error(), `migration schema "locked" is not accessible`) { + t.Fatalf("RunUpAsRole error = %v", err) + } + if testTableExists(t, dsn, "public."+source.TrackingTable) || testTableExists(t, dsn, "public.migration_test") { + t.Fatal("migration wrote into public") + } +} + +func TestPgvectorSchemaMismatchFailsBeforeMigrations(t *testing.T) { + dsn := startTestDB(t) + sources := BuiltinSourcesInSchema(true, "tenant_one", "extensions") + if err := RunUp(t.Context(), dsn, sources); err == nil || !strings.Contains(err.Error(), `installed in schema "public", expected "extensions"`) { + t.Fatalf("RunUp error = %v", err) + } + if err := VerifyMigrated(t.Context(), dsn, sources); err == nil || !strings.Contains(err.Error(), `installed in schema "public", expected "extensions"`) { + t.Fatalf("VerifyMigrated error = %v", err) + } + if testTableExists(t, dsn, "tenant_one."+coreTrackingTable) { + t.Fatal("core migration ran before the pgvector schema precheck") + } +} + func TestBuiltinMigrationsRoundTrip(t *testing.T) { dsn := startTestDB(t) - sources := BuiltinSources(true) + sources := BuiltinSourcesInSchema(true, "public", "public") if err := RunUp(context.Background(), dsn, sources); err != nil { t.Fatalf("initial RunUp: %v", err) @@ -439,6 +529,7 @@ func TestValidateSources(t *testing.T) { {Name: "", TrackingTable: valid.TrackingTable, FS: valid.FS, Dir: valid.Dir}, {Name: "test", TrackingTable: "Bad-Table", FS: valid.FS, Dir: valid.Dir}, {Name: "test", Schema: "Bad-Schema", TrackingTable: valid.TrackingTable, FS: valid.FS, Dir: valid.Dir}, + {Name: "test", VectorSchema: "public", TrackingTable: valid.TrackingTable, FS: valid.FS, Dir: valid.Dir}, } for _, source := range tests { if err := validateSources([]Source{source}); err == nil { @@ -464,17 +555,51 @@ func TestBuiltinTrackingTables(t *testing.T) { if sources[1].TrackingTable != vectorTrackingTable { t.Fatalf("vector source = %+v", sources[1]) } + if sources[0].Schema != "kagent" || sources[1].VectorSchema != "extensions" { + t.Fatalf("default schemas = %q, %q", sources[0].Schema, sources[1].VectorSchema) + } +} + +func TestBuiltinSourcesInSchema(t *testing.T) { + sources := BuiltinSourcesInSchema(true, "tenant_schema", "shared_extensions") + for _, source := range sources { + if source.Schema != "tenant_schema" { + t.Fatalf("source %q schema = %q", source.Name, source.Schema) + } + if source.VectorSchema != "shared_extensions" { + t.Fatalf("source %q vector schema = %q", source.Name, source.VectorSchema) + } + } + defaults := BuiltinSourcesInSchema(true, "", "") + if defaults[0].Schema != "kagent" || defaults[1].VectorSchema != "extensions" { + t.Fatalf("default schemas = %q, %q", defaults[0].Schema, defaults[1].VectorSchema) + } } func TestWithSearchPath(t *testing.T) { - got, err := withSearchPath("postgres://u:p@host/db?sslmode=disable", "tenant_1") + got, err := withSearchPath("postgres://u:p@host/db?sslmode=disable", "tenant_1", "extensions") + if err != nil { + t.Fatal(err) + } + parsed, err := url.Parse(got) + if err != nil { + t.Fatal(err) + } + if parsed.Query().Get("search_path") != `"tenant_1"` || parsed.Query().Get("kagent.vector_schema") != "extensions" || parsed.Query().Get("sslmode") != "disable" { + t.Fatalf("URL query = %q", parsed.RawQuery) + } + got, err = withSearchPath("postgres://u:p@host/db?kagent.vector_schema=other", "", "public") + if err != nil { + t.Fatal(err) + } + parsed, err = url.Parse(got) if err != nil { t.Fatal(err) } - if !strings.Contains(got, "search_path=tenant_1") || !strings.Contains(got, "sslmode=disable") { - t.Fatalf("URL = %q", got) + if parsed.Query().Has("search_path") || parsed.Query().Get("kagent.vector_schema") != "public" { + t.Fatalf("URL query = %q", parsed.RawQuery) } - if _, err := withSearchPath("mysql://host/db", "tenant_1"); err == nil { + if _, err := withSearchPath("mysql://host/db", "tenant_1", "extensions"); err == nil { t.Fatal("withSearchPath accepted MySQL") } } diff --git a/go/core/pkg/migrations/vector/000001_initial.sql b/go/core/pkg/migrations/vector/000001_initial.sql index c99d3dfa10..ad0a3556b5 100644 --- a/go/core/pkg/migrations/vector/000001_initial.sql +++ b/go/core/pkg/migrations/vector/000001_initial.sql @@ -2,14 +2,11 @@ -- Kagent 1.0 vector baseline. -CREATE EXTENSION IF NOT EXISTS vector; - CREATE TABLE memory ( id TEXT PRIMARY KEY DEFAULT gen_random_uuid(), agent_name TEXT, user_id TEXT, content TEXT, - embedding vector(768), metadata TEXT, created_at TIMESTAMPTZ, expires_at TIMESTAMPTZ, @@ -17,7 +14,17 @@ CREATE TABLE memory ( ); CREATE INDEX idx_memory_agent_user ON memory(agent_name, user_id); CREATE INDEX idx_memory_expires_at ON memory(expires_at); -CREATE INDEX idx_memory_embedding_hnsw ON memory USING hnsw (embedding vector_cosine_ops); + +-- +goose StatementBegin +DO $vector$ +DECLARE + vector_schema text := COALESCE(NULLIF(current_setting('kagent.vector_schema', true), ''), 'extensions'); +BEGIN + EXECUTE format('ALTER TABLE memory ADD COLUMN embedding %I.vector(768)', vector_schema); + EXECUTE format('CREATE INDEX idx_memory_embedding_hnsw ON memory USING hnsw (embedding %I.vector_cosine_ops)', vector_schema); +END +$vector$; +-- +goose StatementEnd -- +goose Down diff --git a/go/core/test/upgrade/roundtrip_test.go b/go/core/test/upgrade/roundtrip_test.go index 7b84c453b5..fa9b71bf45 100644 --- a/go/core/test/upgrade/roundtrip_test.go +++ b/go/core/test/upgrade/roundtrip_test.go @@ -114,7 +114,7 @@ func applyEmbeddedMigrations(t *testing.T, env upgradeEnv, database string, vect defer stop() url := fmt.Sprintf("postgres://kagent:kagent@127.0.0.1:%d/%s?sslmode=disable", localPort, database) - require.NoError(t, migrations.RunUp(t.Context(), url, migrations.BuiltinSources(vectorEnabled)), + require.NoError(t, migrations.RunUp(t.Context(), url, migrations.BuiltinSourcesInSchema(vectorEnabled, "public", "public")), "apply embedded migrations to database %s", database) } @@ -125,7 +125,7 @@ func migrateEmbeddedSourcesTo(t *testing.T, env upgradeEnv, targets map[string]i defer stop() url := fmt.Sprintf("postgres://kagent:kagent@127.0.0.1:%d/kagent?sslmode=disable", localPort) - for _, source := range slices.Backward(migrations.BuiltinSources(vectorEnabled)) { + for _, source := range slices.Backward(migrations.BuiltinSourcesInSchema(vectorEnabled, "public", "public")) { target, ok := targets[source.Name] require.True(t, ok, "missing rollback target for migration source %s", source.Name) err := migrations.WithProvider(t.Context(), url, source, func(provider *goose.Provider) error { diff --git a/helm/README.md b/helm/README.md index 1da4f16e73..fc34ea83e3 100644 --- a/helm/README.md +++ b/helm/README.md @@ -21,6 +21,127 @@ helm install kagent ./helm/kagent/ --namespace kagent --set providers.default=an helm install kagent ./helm/kagent/ --namespace kagent --set providers.default=azureOpenAI --set providers.azureOpenAI.apiKey=your-openai-api-key ``` +### Substrate PostgreSQL + +The default install uses one PostgreSQL instance and one `kagent` database. +Kagent uses the `kagent` schema by default. Substrate uses the `substrate` schema. +This identity layout requires a fresh database; upgrading an existing database to it is unsupported. +When vectors are enabled, `database.postgres.vectorSchema` names the one schema +that holds the shared pgvector extension (default `extensions`). All Kagent installs +using the same database must select that schema. For an external database, +install pgvector there before running migrations, or set `vectorSchema` to its +existing location (such as `public`). Grant the application role +`USAGE` on the extension schema. Set `POSTGRES_VECTOR_SCHEMA` to the same value +when running the database CLI outside the chart. +When separate from Kagent's table schema, the pgvector schema stays out of its +normal SQL search path. Kagent qualifies its pgvector type, index operator +class, and cosine operator references. A shared `extensions` schema may also +contain other applications' objects. Grant Kagent +`USAGE` on that schema; reserve `CREATE` for trusted administrators. + +With `database.postgres.bundled.bootstrap=true`, each controller pod runs identity +bootstrap on every start, before migrations. Repeated runs create only missing +identities and do not reset existing passwords. If `database.postgres.vectorEnabled` +changes from `false` to `true`, the next start installs the `vector` extension in +`vectorSchema` and then applies pending vector migrations. The default bundled +PostgreSQL image does not include pgvector; select an image with pgvector installed +before enabling vectors. With bootstrap disabled or an external database, install +the extension yourself before enabling vectors. + +The install creates separate users and group roles: + +| Product access | User | Group role | +| --- | --- | --- | +| Kagent | `kagent_user` | `kagent_owner` | +| Substrate owner | `substrate_admin_user` | `substrate_owner` | +| Substrate read/write | `substrate_readwrite_user` | `substrate_readwrite` | + +Enable Substrate to use this layout: + +```yaml +substrate: + enabled: true +``` + +The chart creates `postgres-admin` for the bundled database. Each control plane uses this Secret before it runs migrations. +If you supply another administrator Secret, set both `database.postgres.bundled.adminSecretRef` and `substrate.postgres.adminSecretRef` to the same name and keys. + +The chart also creates three application Secrets. Its default administrator and application passwords are fixed, published values. This bundled bootstrap setup is for development and evaluation, not production. For production, provision unique users and permissions externally, provide connection Secrets, and disable bootstrap. For Substrate, Kagent passes the bundled PostgreSQL Service address and the `kagent` database name to connection-string templates owned by the Substrate chart. Those templates supply Substrate's fixed usernames and passwords. + +For an external database, create the users, roles, schemas, and grants yourself, then provide three application connection Secrets: + +```yaml +database: + postgres: + secretRef: + name: kagent-postgres + key: connectionString + bundled: + enabled: false +substrate: + enabled: true + postgres: + enabled: false + readWriteConnectionStringSecretRef: + name: substrate-postgres-readwrite + key: readWriteConnectionString + ownerConnectionStringSecretRef: + name: substrate-postgres-owner + key: ownerConnectionString + bootstrap: false +``` + +Bundled bootstrap creates only the fixed users, using the same fixed development credentials compiled into each product and rendered into its connection Secrets. It also creates group roles, schemas, memberships, and grants. It does not change existing passwords. Bootstrap rejects a connection Secret whose credentials differ from those fixed defaults. + +To use externally created users with the bundled database, first create the replacement users and connection Secrets. Then set `database.postgres.bundled.bootstrap=false` and provide `database.postgres.secretRef.name` in the same upgrade. If Substrate is enabled, set `substrate.postgres.bootstrap=false` and provide its owner and read/write Secret references too. The bundled PostgreSQL pod still uses its administrator Secret; neither control plane resets the original users' passwords. + +For a BYO database, create these objects before installation. Keep migrations enabled. +Set `database.postgres.role` to the Kagent owner role and, when Substrate is +enabled, set `substrate.postgres.ownerRole` and `substrate.postgres.readWriteRole` +to the roles you provisioned. Use distinct role names and table schemas for +separate installs sharing one database. Give each install separate logins and +grant each login membership only in its install's roles. Bundled bootstrap uses +the fixed role names and requires the default values. + +The application uses the same identity SQL that operators can run: [Kagent identity SQL](../go/core/pkg/migrations/identity/bootstrap.sql) and `cmd/ateapi/internal/store/atepg/identity.sql` in the Substrate repository. These files sit beside the migration sources but run separately, as an administrator. Set the transaction-local parameters listed at the top of each file before running it. +For manual provisioning with custom chart role names, set +`kagent.bootstrap_owner_role`, `substrate.bootstrap_owner_role`, and +`substrate.bootstrap_readwrite_role` as transaction-local settings before +running the applicable SQL file. They default to the fixed development names; +the bundled binary bootstrap passes those fixed names explicitly. + +#### Credential rotation + +An outside process rotates credentials. First, create a new user and grant the applicable group role. + +Next, update the connection Secret. Kagent and Substrate read the Secret before each new physical connection. + +Set each pool lifetime to limit old connection use. Keep both users valid during Secret projection and connection replacement. + +A host, port, fallback target, or database change requires a restart. + +Kagent 1.x removes `database.postgres.url` and `database.postgres.urlFile`. Replace either value: + +```yaml +database: + postgres: + url: postgresql://user:password@database.example/kagent +``` + +with: + +```yaml +database: + postgres: + secretRef: + name: postgres-connection + key: connectionString +``` + +This supports externally rotated Secret values. Minting an RDS IAM token in +process on every connection is separate work and requires equivalent hooks in +both Kagent and Substrate. + ### Using Make ```bash diff --git a/helm/kagent/templates/NOTES.txt b/helm/kagent/templates/NOTES.txt index 1eb51f0865..5f9afa86f1 100644 --- a/helm/kagent/templates/NOTES.txt +++ b/helm/kagent/templates/NOTES.txt @@ -62,26 +62,15 @@ DOCUMENTATION: {{- end }} {{ if .Values.database.postgres.bundled.enabled -}} ################################################################################ -{{- if and (eq .Values.database.postgres.url "") (eq .Values.database.postgres.urlFile "") }} # WARNING: BUNDLED DATABASE IN USE # ################################################################################ The bundled PostgreSQL instance is enabled. It is intended for development and evaluation only, not suitable for production use. Data may be lost if the pod is restarted or rescheduled. - To use an external database, set: - database.postgres.url= or database.postgres.urlFile= -{{- else }} -# NOTE: BUNDLED DATABASE DEPLOYED BUT NOT IN USE BY CONTROLLER # -################################################################################ - The bundled PostgreSQL pod is running, but the controller is connected to an - external database (database.postgres.url or database.postgres.urlFile is set). - - To connect the controller to the bundled instance instead, unset url/urlFile: - database.postgres.url="" - To stop deploying the bundled pod entirely, set: + To use an external database, set these values: database.postgres.bundled.enabled=false -{{- end }} + database.postgres.secretRef.name= {{- end }} {{- if .Values.database.postgres.skipMigrations }} ################################################################################ diff --git a/helm/kagent/templates/_helpers.tpl b/helm/kagent/templates/_helpers.tpl index a3491fbcec..6194ced2ee 100644 --- a/helm/kagent/templates/_helpers.tpl +++ b/helm/kagent/templates/_helpers.tpl @@ -286,11 +286,13 @@ Bundled PostgreSQL image - constructs the full image reference from registry/rep {{- printf "%s:%s" (join "/" $parts) $pg.image.tag -}} {{- end -}} -{{/* -Password secret name - returns the chart-managed Secret name for POSTGRES_PASSWORD. -*/}} -{{- define "kagent.passwordSecretName" -}} -{{- printf "%s-postgresql" (include "kagent.fullname" .) -}} +{{/* PostgreSQL bootstrap helpers. */}} +{{- define "kagent.postgres.connectionSecretName" -}} +{{- .Values.database.postgres.secretRef.name | default "kagent-postgres" -}} +{{- end -}} + +{{- define "kagent.postgres.adminSecretName" -}} +{{- .Values.database.postgres.bundled.adminSecretRef.name | default "postgres-admin" -}} {{- end -}} {{/* Public A2A endpoint advertised by AgentInstance Agent Cards. */}} diff --git a/helm/kagent/templates/controller-configmap.yaml b/helm/kagent/templates/controller-configmap.yaml index addbcebea8..5b300c8547 100644 --- a/helm/kagent/templates/controller-configmap.yaml +++ b/helm/kagent/templates/controller-configmap.yaml @@ -51,6 +51,9 @@ data: {{- end }} DATABASE_VECTOR_ENABLED: {{ .Values.database.postgres.vectorEnabled | quote }} SKIP_MIGRATIONS: {{ .Values.database.postgres.skipMigrations | default false | quote }} + POSTGRES_DATABASE_ROLE: {{ .Values.database.postgres.role | quote }} + POSTGRES_DATABASE_SCHEMA: {{ .Values.database.postgres.schema | quote }} + POSTGRES_VECTOR_SCHEMA: {{ .Values.database.postgres.vectorSchema | quote }} {{- with .Values.database.postgres.pool }} {{- if and (hasKey . "maxConns") (ne .maxConns nil) }} DB_MAX_CONNS: {{ .maxConns | quote }} diff --git a/helm/kagent/templates/controller-deployment.yaml b/helm/kagent/templates/controller-deployment.yaml index 9de69c00b1..c262fe6a15 100644 --- a/helm/kagent/templates/controller-deployment.yaml +++ b/helm/kagent/templates/controller-deployment.yaml @@ -1,3 +1,30 @@ +{{- $databaseConnectionStringSecretRef := .Values.database.postgres.secretRef | default dict -}} +{{- $bundled := .Values.database.postgres.bundled -}} +{{- $bootstrapEnabled := and $bundled.enabled $bundled.bootstrap -}} +{{- $databaseSecretVolumeName := "postgres-connection" -}} +{{- $databaseSecretMountPath := "/var/run/secrets/kagent/postgres" -}} +{{- $databaseSecretFileName := "connection-string" -}} +{{- if hasKey .Values.database.postgres "urlFile" -}} +{{- fail "database.postgres.urlFile has been removed; use database.postgres.secretRef.{name,key}" -}} +{{- end -}} +{{- if hasKey .Values.database.postgres "url" -}} +{{- fail "database.postgres.url has been removed; use database.postgres.secretRef.{name,key}" -}} +{{- end -}} +{{- if hasKey .Values.database.postgres "bootstrap" -}} +{{- fail "database.postgres.bootstrap moved to database.postgres.bundled.bootstrap" -}} +{{- end -}} +{{- if not (kindIs "bool" $bundled.bootstrap) -}} +{{- fail "database.postgres.bundled.bootstrap must be true or false" -}} +{{- end -}} +{{- if and $bootstrapEnabled (ne .Values.database.postgres.role "kagent_owner") -}} +{{- fail "database.postgres.role must be kagent_owner while bundled bootstrap is enabled" -}} +{{- end -}} +{{- if and $bootstrapEnabled (get $databaseConnectionStringSecretRef "name") -}} +{{- fail "database.postgres.secretRef.name requires database.postgres.bundled.bootstrap=false" -}} +{{- end -}} +{{- if and (not $bootstrapEnabled) (not (get $databaseConnectionStringSecretRef "name")) -}} +{{- fail "database.postgres.secretRef.name is required when bootstrap is disabled" -}} +{{- end -}} apiVersion: apps/v1 kind: Deployment metadata: @@ -35,8 +62,27 @@ spec: {{- toYaml . | nindent 8 }} {{- end }} serviceAccountName: {{ include "kagent.fullname" . }}-controller - {{- if or (gt (len .Values.controller.volumes) 0) (and .Values.controller.substrate .Values.controller.substrate.enabled) }} volumes: + - name: {{ $databaseSecretVolumeName }} + projected: + sources: + - secret: + name: {{ include "kagent.postgres.connectionSecretName" . }} + items: + - key: {{ get $databaseConnectionStringSecretRef "key" | default "connectionString" }} + path: {{ $databaseSecretFileName }} + {{- if $bootstrapEnabled }} + - name: postgres-admin + projected: + sources: + - secret: + name: {{ include "kagent.postgres.adminSecretName" . }} + items: + - key: {{ .Values.database.postgres.bundled.adminSecretRef.usernameKey }} + path: username + - key: {{ .Values.database.postgres.bundled.adminSecretRef.passwordKey }} + path: password + {{- end }} {{- if and .Values.controller.substrate .Values.controller.substrate.enabled }} - name: substrate-servicedns projected: @@ -58,7 +104,6 @@ spec: {{- with .Values.controller.volumes }} {{- toYaml . | nindent 6 }} {{- end }} - {{- end }} {{- with .Values.controller.nodeSelector }} nodeSelector: {{- toYaml . | nindent 8 }} @@ -98,22 +143,15 @@ spec: - name: AUTH_USER_ID_CLAIM value: {{ .Values.controller.auth.userIdClaim | quote }} {{- end }} - {{- if .Values.database.postgres.urlFile }} - - name: POSTGRES_DATABASE_URL_FILE - value: {{ .Values.database.postgres.urlFile | quote }} - {{- else if .Values.database.postgres.url }} - name: POSTGRES_DATABASE_URL - value: {{ .Values.database.postgres.url | quote }} - {{- else if .Values.database.postgres.bundled.enabled }} - - name: POSTGRES_PASSWORD - valueFrom: - secretKeyRef: - name: {{ include "kagent.passwordSecretName" . }} - key: POSTGRES_PASSWORD - - name: POSTGRES_DATABASE_URL - value: {{ printf "postgres://kagent:$(POSTGRES_PASSWORD)@%s.%s.svc:5432/kagent?sslmode=disable" (include "kagent.postgresqlServiceName" .) (include "kagent.namespace" .) | quote }} - {{- else }} - {{ fail "No database connection configured. Set database.postgres.url, database.postgres.urlFile, or enable database.postgres.bundled." }} + value: {{ printf "@file:%s/%s" $databaseSecretMountPath $databaseSecretFileName | quote }} + - name: KAGENT_DATABASE_BOOTSTRAP + value: {{ $bootstrapEnabled | quote }} + {{- if $bootstrapEnabled }} + - name: POSTGRES_ADMIN_USERNAME_FILE + value: /var/run/secrets/kagent/postgres-admin/username + - name: POSTGRES_ADMIN_PASSWORD_FILE + value: /var/run/secrets/kagent/postgres-admin/password {{- end }} {{- if include "kagent.controller.metricsEnabled" . }} - name: METRICS_BIND_ADDRESS @@ -194,8 +232,15 @@ spec: port: http periodSeconds: 30 {{- end }} - {{- if or (gt (len .Values.controller.volumeMounts) 0) (and .Values.controller.substrate .Values.controller.substrate.enabled) }} volumeMounts: + - name: {{ $databaseSecretVolumeName }} + mountPath: {{ $databaseSecretMountPath }} + readOnly: true + {{- if $bootstrapEnabled }} + - name: postgres-admin + mountPath: /var/run/secrets/kagent/postgres-admin + readOnly: true + {{- end }} {{- if and .Values.controller.substrate .Values.controller.substrate.enabled }} - name: substrate-servicedns mountPath: /run/substrate-servicedns @@ -207,4 +252,3 @@ spec: {{- with .Values.controller.volumeMounts }} {{- toYaml . | nindent 12 }} {{- end }} - {{- end }} diff --git a/helm/kagent/templates/postgresql-secret.yaml b/helm/kagent/templates/postgresql-secret.yaml index 3adb5b3c4e..524acdd06a 100644 --- a/helm/kagent/templates/postgresql-secret.yaml +++ b/helm/kagent/templates/postgresql-secret.yaml @@ -1,13 +1,76 @@ -{{- if .Values.database.postgres.bundled.enabled }} +{{- $postgres := .Values.database.postgres -}} +{{- $bootstrapEnabled := and $postgres.bundled.enabled $postgres.bundled.bootstrap -}} +{{- $host := printf "%s.%s.svc" (include "kagent.postgresqlServiceName" .) (include "kagent.namespace" .) -}} +{{- if and $postgres.bundled.enabled (not $postgres.bundled.adminSecretRef.name) }} apiVersion: v1 kind: Secret metadata: - name: {{ include "kagent.passwordSecretName" . }} + name: {{ include "kagent.postgres.adminSecretName" . }} namespace: {{ include "kagent.namespace" . }} labels: {{- include "kagent.labels" . | nindent 4 }} app.kubernetes.io/component: database type: Opaque -data: - POSTGRES_PASSWORD: {{ "kagent" | b64enc | quote }} +stringData: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: postgres +--- +{{- end }} +{{- if and $postgres.bundled.enabled $bootstrapEnabled (not $postgres.secretRef.name) }} +apiVersion: v1 +kind: Secret +metadata: + name: {{ include "kagent.postgres.connectionSecretName" . }} + namespace: {{ include "kagent.namespace" . }} + labels: + {{- include "kagent.labels" . | nindent 4 }} + app.kubernetes.io/component: database +type: Opaque +stringData: + {{ $postgres.secretRef.key | default "connectionString" }}: {{ printf "postgresql://kagent_user:kagent@%s:5432/kagent?sslmode=disable" $host | quote }} +--- +{{- end }} +{{- $substratePostgres := .Values.substrate.postgres | default dict -}} +{{- $substrateBootstrap := get $substratePostgres "bootstrap" -}} +{{- $readWriteRef := get $substratePostgres "readWriteConnectionStringSecretRef" | default dict -}} +{{- $ownerRef := get $substratePostgres "ownerConnectionStringSecretRef" | default dict -}} +{{- $substrateAdminRef := get $substratePostgres "adminSecretRef" | default dict -}} +{{- if and .Values.substrate.enabled $substrateBootstrap (not $postgres.bundled.enabled) -}} +{{- fail "substrate.postgres.bootstrap requires database.postgres.bundled.enabled=true" -}} +{{- end -}} +{{- if and .Values.substrate.enabled $postgres.bundled.enabled $substrateBootstrap (ne $substratePostgres.database "kagent") -}} +{{- fail "substrate.postgres.database must be kagent when sharing Kagent's bundled PostgreSQL" -}} +{{- end -}} +{{- if and .Values.substrate.enabled $postgres.bundled.enabled $substrateBootstrap (or (ne (get $substrateAdminRef "name") (include "kagent.postgres.adminSecretName" .)) (ne (get $substrateAdminRef "usernameKey") $postgres.bundled.adminSecretRef.usernameKey) (ne (get $substrateAdminRef "passwordKey") $postgres.bundled.adminSecretRef.passwordKey)) -}} +{{- fail "substrate.postgres.adminSecretRef must match database.postgres.bundled.adminSecretRef when sharing bundled PostgreSQL" -}} +{{- end -}} +{{- if and .Values.substrate.enabled $postgres.bundled.enabled $substrateBootstrap (or (ne (get $readWriteRef "name") "substrate-postgres-readwrite") (ne (get $ownerRef "name") "substrate-postgres-owner")) -}} +{{- fail "substrate.postgres.bootstrap requires the chart-managed Substrate application Secret names; disable bootstrap for operator-managed Secrets" -}} +{{- end -}} +{{- if and .Values.substrate.enabled $postgres.bundled.enabled $substrateBootstrap (eq (get $readWriteRef "name") "substrate-postgres-readwrite") }} +apiVersion: v1 +kind: Secret +metadata: + name: {{ get $readWriteRef "name" | default "substrate-postgres-readwrite" }} + namespace: {{ include "kagent.namespace" . }} + labels: + {{- include "kagent.labels" . | nindent 4 }} + app.kubernetes.io/component: database +type: Opaque +stringData: + {{ get $readWriteRef "key" | default "readWriteConnectionString" }}: {{ include "substrate.postgres.readWriteConnectionString" (dict "host" $host "database" $substratePostgres.database "params" "sslmode=disable") | quote }} +--- +{{- end }} +{{- if and .Values.substrate.enabled $postgres.bundled.enabled $substrateBootstrap (eq (get $ownerRef "name") "substrate-postgres-owner") }} +apiVersion: v1 +kind: Secret +metadata: + name: {{ get $ownerRef "name" | default "substrate-postgres-owner" }} + namespace: {{ include "kagent.namespace" . }} + labels: + {{- include "kagent.labels" . | nindent 4 }} + app.kubernetes.io/component: database +type: Opaque +stringData: + {{ get $ownerRef "key" | default "ownerConnectionString" }}: {{ include "substrate.postgres.ownerConnectionString" (dict "host" $host "database" $substratePostgres.database "params" "sslmode=disable") | quote }} {{- end }} diff --git a/helm/kagent/templates/postgresql.yaml b/helm/kagent/templates/postgresql.yaml index 0f735c1aa2..e85ced44cc 100644 --- a/helm/kagent/templates/postgresql.yaml +++ b/helm/kagent/templates/postgresql.yaml @@ -87,22 +87,23 @@ spec: - name: POSTGRES_DB value: "kagent" - name: POSTGRES_USER - value: "kagent" + valueFrom: + secretKeyRef: + name: {{ include "kagent.postgres.adminSecretName" . }} + key: {{ .Values.database.postgres.bundled.adminSecretRef.usernameKey }} - name: POSTGRES_PASSWORD valueFrom: secretKeyRef: - name: {{ include "kagent.passwordSecretName" . }} - key: POSTGRES_PASSWORD + name: {{ include "kagent.postgres.adminSecretName" . }} + key: {{ .Values.database.postgres.bundled.adminSecretRef.passwordKey }} - name: PGDATA value: /var/lib/postgresql/data/pgdata livenessProbe: exec: command: - - pg_isready - - -U - - kagent - - -d - - kagent + - sh + - -c + - pg_isready -U "$POSTGRES_USER" -d "$POSTGRES_DB" initialDelaySeconds: 20 periodSeconds: 10 timeoutSeconds: 5 @@ -111,11 +112,9 @@ spec: readinessProbe: exec: command: - - pg_isready - - -U - - kagent - - -d - - kagent + - sh + - -c + - pg_isready -U "$POSTGRES_USER" -d "$POSTGRES_DB" initialDelaySeconds: 5 periodSeconds: 5 timeoutSeconds: 3 diff --git a/helm/kagent/tests/controller-deployment_test.yaml b/helm/kagent/tests/controller-deployment_test.yaml index c1e26e9bc3..762570f834 100644 --- a/helm/kagent/tests/controller-deployment_test.yaml +++ b/helm/kagent/tests/controller-deployment_test.yaml @@ -18,6 +18,22 @@ tests: - hasDocuments: count: 1 + - it: should reject operator-managed Kagent Secrets during bootstrap + template: controller-deployment.yaml + set: + database.postgres.secretRef.name: custom-kagent + asserts: + - failedTemplate: + errorMessage: database.postgres.secretRef.name requires database.postgres.bundled.bootstrap=false + + - it: should reject custom Kagent roles during bootstrap + template: controller-deployment.yaml + set: + database.postgres.role: tenant_owner + asserts: + - failedTemplate: + errorMessage: database.postgres.role must be kagent_owner while bundled bootstrap is enabled + - it: should render the controller deployment with custom replica count template: controller-deployment.yaml set: @@ -123,6 +139,12 @@ tests: - it: should configure substrate ate-api mTLS when substrate is enabled template: controller-deployment.yaml set: + database: + postgres: + secretRef: + name: external-postgres + bundled: + enabled: false controller: substrate: enabled: true @@ -144,26 +166,23 @@ tests: name: SUBSTRATE_ATE_API_CLIENT_CERT_FILE value: /run/substrate-podidentity/credential-bundle.pem - equal: - path: spec.template.spec.volumes[0].name - value: substrate-servicedns - - equal: - path: spec.template.spec.volumes[0].projected.sources[0].clusterTrustBundle.signerName - value: servicedns.podcert.ate.dev/identity - - equal: - path: spec.template.spec.volumes[1].projected.sources[0].podCertificate.signerName - value: podidentity.podcert.ate.dev/identity - - equal: - path: spec.template.spec.volumes[1].projected.sources[0].podCertificate.credentialBundlePath - value: credential-bundle.pem - - equal: - path: spec.template.spec.containers[0].volumeMounts[0].name + path: spec.template.spec.volumes[1].name value: substrate-servicedns - equal: - path: spec.template.spec.containers[0].volumeMounts[0].mountPath - value: /run/substrate-servicedns - - equal: - path: spec.template.spec.containers[0].volumeMounts[1].mountPath - value: /run/substrate-podidentity + path: spec.template.spec.volumes[2].name + value: substrate-podidentity + - contains: + path: spec.template.spec.containers[0].volumeMounts + content: + name: substrate-servicedns + mountPath: /run/substrate-servicedns + readOnly: true + - contains: + path: spec.template.spec.containers[0].volumeMounts + content: + name: substrate-podidentity + mountPath: /run/substrate-podidentity + readOnly: true - it: should set KAGENT_GATEWAY_URL with computed default value template: controller-configmap.yaml @@ -471,64 +490,238 @@ tests: name: extra-data mountPath: /extra - - it: should not render volumes or volumeMounts sections when none are configured + - it: should always mount the database Secret template: controller-deployment.yaml + set: + database: + postgres: + secretRef: + name: external-postgres + bundled: + enabled: false asserts: - - isNull: - path: spec.template.spec.volumes - - isNull: - path: spec.template.spec.containers[0].volumeMounts + - equal: + path: spec.template.spec.volumes[0].projected.sources[0].secret.name + value: external-postgres + - equal: + path: spec.template.spec.containers[0].volumeMounts[0].name + value: postgres-connection # ============================================================================= # Database Configuration Tests # ============================================================================= - - it: should set POSTGRES_PASSWORD from secret and POSTGRES_DATABASE_URL with bundled connection string by default + - it: should reject a nonboolean bootstrap value template: controller-deployment.yaml + set: + database.postgres.bundled.bootstrap: disabled + asserts: + - failedTemplate: + errorMessage: database.postgres.bundled.bootstrap must be true or false + + - it: should reject the old bootstrap location + template: controller-deployment.yaml + set: + database.postgres.bootstrap: false + asserts: + - failedTemplate: + errorMessage: database.postgres.bootstrap moved to database.postgres.bundled.bootstrap + + - it: should use existing users with bundled PostgreSQL when bootstrap is disabled + template: controller-deployment.yaml + set: + database.postgres.bundled.bootstrap: false + database.postgres.secretRef.name: existing-kagent asserts: - contains: path: spec.template.spec.containers[0].env content: - name: POSTGRES_PASSWORD - valueFrom: - secretKeyRef: - name: RELEASE-NAME-postgresql - key: POSTGRES_PASSWORD + name: KAGENT_DATABASE_BOOTSTRAP + value: "false" + - notExists: + path: spec.template.spec.volumes[?(@.name == "postgres-admin")] + + - it: should require an application Secret when bundled bootstrap is disabled + template: controller-deployment.yaml + set: + database.postgres.bundled.bootstrap: false + asserts: + - failedTemplate: + errorMessage: database.postgres.secretRef.name is required when bootstrap is disabled + + - it: should use the managed Kagent Secret by default + template: controller-deployment.yaml + asserts: - contains: path: spec.template.spec.containers[0].env content: name: POSTGRES_DATABASE_URL - value: "postgres://kagent:$(POSTGRES_PASSWORD)@RELEASE-NAME-postgresql.NAMESPACE.svc:5432/kagent?sslmode=disable" + value: "@file:/var/run/secrets/kagent/postgres/connection-string" + - notExists: + path: spec.template.spec.initContainers + - contains: + path: spec.template.spec.containers[0].env + content: + name: KAGENT_DATABASE_BOOTSTRAP + value: "true" + - contains: + path: spec.template.spec.containers[0].volumeMounts + content: + name: postgres-admin + mountPath: /var/run/secrets/kagent/postgres-admin + readOnly: true - - it: should set POSTGRES_DATABASE_URL with external url when url is set + - it: should use an external database Secret template: controller-deployment.yaml set: database: postgres: - url: "postgres://user:pass@external-host:5432/db" + secretRef: + name: external-postgres + bundled: + enabled: false + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: POSTGRES_DATABASE_URL + value: "@file:/var/run/secrets/kagent/postgres/connection-string" + + - it: should keep the managed Kagent Secret when Substrate is enabled + template: controller-deployment.yaml + set: + substrate: + enabled: true + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: POSTGRES_DATABASE_URL + value: "@file:/var/run/secrets/kagent/postgres/connection-string" + + - it: should keep an explicitly separate Substrate database Secret separate + template: controller-deployment.yaml + set: + substrate: + enabled: true + postgres: + bootstrap: false + readWriteConnectionStringSecretRef: + name: substrate-db asserts: - contains: path: spec.template.spec.containers[0].env content: name: POSTGRES_DATABASE_URL - value: "postgres://user:pass@external-host:5432/db" + value: "@file:/var/run/secrets/kagent/postgres/connection-string" + - notContains: + path: spec.template.spec.containers[0].env + content: + name: POSTGRES_DATABASE_URL + valueFrom: + secretKeyRef: + name: substrate-db + key: connectionString - - it: should set POSTGRES_DATABASE_URL_FILE and omit POSTGRES_PASSWORD when urlFile is set + - it: should read an external database URL from a Secret template: controller-deployment.yaml set: database: postgres: - urlFile: "/var/secrets/db-url" + bundled: + enabled: false + secretRef: + name: external-postgres + key: url asserts: - contains: path: spec.template.spec.containers[0].env content: - name: POSTGRES_DATABASE_URL_FILE - value: "/var/secrets/db-url" + name: POSTGRES_DATABASE_URL + value: "@file:/var/run/secrets/kagent/postgres/connection-string" + - notContains: + path: spec.template.spec.containers[0].env + content: + name: POSTGRES_DATABASE_URL + valueFrom: + secretKeyRef: + name: external-postgres + key: url + - contains: + path: spec.template.spec.volumes + content: + name: postgres-connection + projected: + sources: + - secret: + name: external-postgres + items: + - key: url + path: connection-string + - contains: + path: spec.template.spec.containers[0].volumeMounts + content: + name: postgres-connection + mountPath: /var/run/secrets/kagent/postgres + readOnly: true + - notExists: + path: spec.template.spec.containers[0].volumeMounts[0].subPath - notContains: path: spec.template.spec.containers[0].env content: name: POSTGRES_PASSWORD + - contains: + path: spec.template.spec.containers[0].env + content: + name: KAGENT_DATABASE_BOOTSTRAP + value: "false" + - notExists: + path: spec.template.spec.volumes[?(@.name == "postgres-admin")] + + - it: should keep Kagent separate from Substrate read/write and owner Secrets + template: controller-deployment.yaml + set: + database: + postgres: + bundled: + enabled: false + secretRef: + name: kagent-db + key: kagent-url + substrate: + enabled: true + postgres: + bootstrap: false + readWriteConnectionStringSecretRef: + name: substrate-readwrite-db + key: readwrite-url + ownerConnectionStringSecretRef: + name: substrate-owner-db + key: owner-url + asserts: + - contains: + path: spec.template.spec.volumes + content: + name: postgres-connection + projected: + sources: + - secret: + name: kagent-db + items: + - key: kagent-url + path: connection-string + - notContains: + path: spec.template.spec.volumes + content: + name: postgres-connection + secret: + secretName: substrate-readwrite-db + - notContains: + path: spec.template.spec.volumes + content: + name: postgres-connection + secret: + secretName: substrate-owner-db - it: should set DATABASE_VECTOR_ENABLED to false by default template: controller-configmap.yaml @@ -548,6 +741,56 @@ tests: path: data.DATABASE_VECTOR_ENABLED value: "true" + - it: should set the default PostgreSQL role + template: controller-configmap.yaml + asserts: + - equal: + path: data.POSTGRES_DATABASE_ROLE + value: kagent_owner + + - it: should set a BYO PostgreSQL role + template: controller-configmap.yaml + set: + database.postgres.bundled.bootstrap: false + database.postgres.secretRef.name: tenant-postgres + database.postgres.role: tenant_owner + asserts: + - equal: + path: data.POSTGRES_DATABASE_ROLE + value: tenant_owner + + - it: should use the kagent PostgreSQL schema by default + template: controller-configmap.yaml + asserts: + - equal: + path: data.POSTGRES_DATABASE_SCHEMA + value: kagent + + - it: should allow a custom PostgreSQL schema + template: controller-configmap.yaml + set: + database.postgres.schema: kagent_custom + asserts: + - equal: + path: data.POSTGRES_DATABASE_SCHEMA + value: kagent_custom + + - it: should use the extensions schema for pgvector by default + template: controller-configmap.yaml + asserts: + - equal: + path: data.POSTGRES_VECTOR_SCHEMA + value: extensions + + - it: should allow an existing pgvector extension in public + template: controller-configmap.yaml + set: + database.postgres.vectorSchema: public + asserts: + - equal: + path: data.POSTGRES_VECTOR_SCHEMA + value: public + - it: should not set DB pool env vars by default template: controller-configmap.yaml asserts: @@ -590,75 +833,25 @@ tests: - notExists: path: data.POSTGRES_DATABASE_URL - - it: should not set POSTGRES_PASSWORD when url is set and bundled is enabled + - it: should reject the removed url value template: controller-deployment.yaml set: database: postgres: url: "postgres://user:pass@external-host:5432/db" - bundled: - enabled: true - asserts: - - notContains: - path: spec.template.spec.containers[0].env - content: - name: POSTGRES_PASSWORD - - - it: should not set POSTGRES_PASSWORD when url is set and bundled is disabled - template: controller-deployment.yaml - set: - database: - postgres: - url: "postgres://user:pass@external-host:5432/db" - bundled: - enabled: false - asserts: - - notContains: - path: spec.template.spec.containers[0].env - content: - name: POSTGRES_PASSWORD - - - it: should set POSTGRES_DATABASE_URL_FILE and omit POSTGRES_PASSWORD when urlFile and bundled are both enabled - template: controller-deployment.yaml - set: - database: - postgres: - urlFile: "/var/secrets/db-url" - bundled: - enabled: true asserts: - - contains: - path: spec.template.spec.containers[0].env - content: - name: POSTGRES_DATABASE_URL_FILE - value: "/var/secrets/db-url" - - notContains: - path: spec.template.spec.containers[0].env - content: - name: POSTGRES_PASSWORD - - notContains: - path: spec.template.spec.containers[0].env - content: - name: POSTGRES_DATABASE_URL + - failedTemplate: + errorMessage: "database.postgres.url has been removed; use database.postgres.secretRef.{name,key}" - - it: should set external POSTGRES_DATABASE_URL and omit POSTGRES_PASSWORD when url and bundled are both enabled + - it: should reject the removed urlFile value template: controller-deployment.yaml set: database: postgres: - url: "postgres://user:pass@external-host:5432/db" - bundled: - enabled: true + urlFile: /var/secrets/db-url asserts: - - contains: - path: spec.template.spec.containers[0].env - content: - name: POSTGRES_DATABASE_URL - value: "postgres://user:pass@external-host:5432/db" - - notContains: - path: spec.template.spec.containers[0].env - content: - name: POSTGRES_PASSWORD + - failedTemplate: + errorMessage: "database.postgres.urlFile has been removed; use database.postgres.secretRef.{name,key}" - it: should use default httpGet startup probe template: controller-deployment.yaml diff --git a/helm/kagent/tests/postgresql_test.yaml b/helm/kagent/tests/postgresql_test.yaml index 5d6d40a018..fea2e87ed7 100644 --- a/helm/kagent/tests/postgresql_test.yaml +++ b/helm/kagent/tests/postgresql_test.yaml @@ -3,517 +3,402 @@ templates: - postgresql.yaml - postgresql-secret.yaml tests: - # ============================================================================= - # bundled mode (default — url and urlFile both empty, bundled.enabled true) - # ============================================================================= - - - it: should render ServiceAccount, PVC, Deployment, and Service when bundled is enabled + - it: should render the bundled PostgreSQL resources template: postgresql.yaml asserts: - hasDocuments: count: 4 - - it: should not render any resources when bundled is disabled + - it: should not render PostgreSQL for an external database template: postgresql.yaml set: - database: - postgres: - bundled: - enabled: false + database.postgres.bundled.enabled: false asserts: - hasDocuments: count: 0 - - it: should still render resources when url is set and bundled is enabled - template: postgresql.yaml - set: - database: - postgres: - url: "postgres://user:pass@external-host:5432/db" - asserts: - - hasDocuments: - count: 4 - - - it: should still render resources when urlFile is set and bundled is enabled - template: postgresql.yaml - set: - database: - postgres: - urlFile: "/var/secrets/db-url" - asserts: - - hasDocuments: - count: 4 - - - - it: should render PVC with correct storage size - template: postgresql.yaml - documentIndex: 1 - asserts: - - isKind: - of: PersistentVolumeClaim - - equal: - path: spec.resources.requests.storage - value: "500Mi" - - - it: should render PVC with custom storage size - template: postgresql.yaml - documentIndex: 1 - set: - database: - postgres: - bundled: - storage: 10Gi - asserts: - - equal: - path: spec.resources.requests.storage - value: "10Gi" - - - it: should not set storageClassName on PVC by default - template: postgresql.yaml - documentIndex: 1 - asserts: - - isKind: - of: PersistentVolumeClaim - - notExists: - path: spec.storageClassName - - - it: should set storageClassName on PVC when specified - template: postgresql.yaml - documentIndex: 1 - set: - database: - postgres: - bundled: - storageClassName: "my-storage-class" - asserts: - - isKind: - of: PersistentVolumeClaim - - equal: - path: spec.storageClassName - value: "my-storage-class" - - - it: should render Deployment with default pgvector image - template: postgresql.yaml - documentIndex: 2 - asserts: - - isKind: - of: Deployment - - equal: - path: spec.template.spec.containers[0].image - value: docker.io/library/postgres:18.6-alpine3.23 - - - it: should render Deployment with custom image - template: postgresql.yaml - documentIndex: 2 - set: - database: - postgres: - bundled: - image: - registry: my-registry.example.com - repository: myorg - name: postgres - tag: "15" - asserts: - - equal: - path: spec.template.spec.containers[0].image - value: my-registry.example.com/myorg/postgres:15 - - - it: should omit empty repository segment in image - template: postgresql.yaml - documentIndex: 2 - set: - database: - postgres: - bundled: - image: - registry: docker.io - repository: "" - name: postgres - tag: "18.3-alpine" - asserts: - - equal: - path: spec.template.spec.containers[0].image - value: docker.io/postgres:18.3-alpine - - notMatchRegex: - path: spec.template.spec.containers[0].image - pattern: "//" - - - it: should read POSTGRES_PASSWORD from chart-managed secret + - it: should use the administrator Secret template: postgresql.yaml documentIndex: 2 asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: POSTGRES_USER + valueFrom: + secretKeyRef: + name: postgres-admin + key: POSTGRES_USER - contains: path: spec.template.spec.containers[0].env content: name: POSTGRES_PASSWORD valueFrom: secretKeyRef: - name: RELEASE-NAME-postgresql + name: postgres-admin key: POSTGRES_PASSWORD - - it: should set POSTGRES_DB and POSTGRES_USER to hardcoded values + - it: should use a custom administrator Secret template: postgresql.yaml documentIndex: 2 + set: + database.postgres.bundled.adminSecretRef: + name: custom-admin + usernameKey: username + passwordKey: password asserts: - contains: path: spec.template.spec.containers[0].env content: - name: POSTGRES_DB - value: "kagent" + name: POSTGRES_USER + valueFrom: + secretKeyRef: + name: custom-admin + key: username - contains: path: spec.template.spec.containers[0].env content: - name: POSTGRES_USER - value: "kagent" + name: POSTGRES_PASSWORD + valueFrom: + secretKeyRef: + name: custom-admin + key: password - - it: should set PGDATA env var + - it: should configure the bundled database template: postgresql.yaml documentIndex: 2 asserts: + - equal: + path: spec.strategy.type + value: Recreate - contains: path: spec.template.spec.containers[0].env content: - name: PGDATA - value: /var/lib/postgresql/data/pgdata - - - it: should have liveness and readiness probes - template: postgresql.yaml - documentIndex: 2 - asserts: + name: POSTGRES_DB + value: kagent + - equal: + path: spec.template.spec.containers[0].image + value: docker.io/library/postgres:18.6-alpine3.23 - isNotNull: path: spec.template.spec.containers[0].livenessProbe - isNotNull: path: spec.template.spec.containers[0].readinessProbe - - it: should render Deployment with default resource requests and limits - template: postgresql.yaml - documentIndex: 2 - asserts: - - equal: - path: spec.template.spec.containers[0].resources.requests.cpu - value: 250m - - equal: - path: spec.template.spec.containers[0].resources.requests.memory - value: 256Mi - - equal: - path: spec.template.spec.containers[0].resources.limits.cpu - value: 500m - - equal: - path: spec.template.spec.containers[0].resources.limits.memory - value: 512Mi - - - it: should render Deployment with custom resources + - it: should use the configured storage template: postgresql.yaml - documentIndex: 2 + documentIndex: 1 set: - database: - postgres: - bundled: - resources: - requests: - cpu: 500m - memory: 512Mi - limits: - cpu: "2" - memory: 2Gi + database.postgres.bundled: + storage: 10Gi + storageClassName: database asserts: - equal: - path: spec.template.spec.containers[0].resources.requests.cpu - value: 500m - - equal: - path: spec.template.spec.containers[0].resources.limits.memory - value: 2Gi - - - it: should render Deployment with Recreate strategy - template: postgresql.yaml - documentIndex: 2 - asserts: + path: spec.resources.requests.storage + value: 10Gi - equal: - path: spec.strategy.type - value: Recreate + path: spec.storageClassName + value: database - - it: should render Deployment with security context - template: postgresql.yaml - documentIndex: 2 + - it: should create the default administrator Secret + template: postgresql-secret.yaml + documentIndex: 0 asserts: - equal: - path: spec.template.spec.securityContext.fsGroup - value: 999 - - equal: - path: spec.template.spec.securityContext.runAsNonRoot - value: true - - equal: - path: spec.template.spec.securityContext.runAsUser - value: 999 - - equal: - path: spec.template.spec.securityContext.runAsGroup - value: 999 + path: metadata.name + value: postgres-admin - equal: - path: spec.template.spec.securityContext.seccompProfile.type - value: RuntimeDefault + path: stringData.POSTGRES_USER + value: postgres - equal: - path: spec.template.spec.containers[0].securityContext.allowPrivilegeEscalation - value: false - - contains: - path: spec.template.spec.containers[0].securityContext.capabilities.drop - content: ALL - - isNull: - path: spec.template.spec.containers[0].securityContext.seccompProfile + path: stringData.POSTGRES_PASSWORD + value: postgres - - it: should allow bundled postgres pod security context override - template: postgresql.yaml - documentIndex: 2 + - it: should keep the administrator Secret when bundled bootstrap is disabled + template: postgresql-secret.yaml set: - database: - postgres: - bundled: - podSecurityContext: - fsGroup: 1001 - runAsUser: 1001 - runAsGroup: 1001 - runAsNonRoot: true - seccompProfile: - type: Localhost - localhostProfile: profiles/postgres.json + database.postgres.bundled.bootstrap: false + database.postgres.secretRef.name: existing-kagent asserts: + - hasDocuments: + count: 1 - equal: - path: spec.template.spec.securityContext.fsGroup - value: 1001 - - equal: - path: spec.template.spec.securityContext.runAsUser - value: 1001 - - equal: - path: spec.template.spec.securityContext.runAsGroup - value: 1001 + path: metadata.name + value: postgres-admin + + - it: should create the default Kagent Secret + template: postgresql-secret.yaml + documentIndex: 1 + asserts: - equal: - path: spec.template.spec.securityContext.seccompProfile.type - value: Localhost + path: metadata.name + value: kagent-postgres - equal: - path: spec.template.spec.securityContext.seccompProfile.localhostProfile - value: profiles/postgres.json + path: stringData.connectionString + value: postgresql://kagent_user:kagent@RELEASE-NAME-postgresql.NAMESPACE.svc:5432/kagent?sslmode=disable - - it: should allow bundled postgres container security context override - template: postgresql.yaml + - it: should create the Substrate read/write Secret + template: postgresql-secret.yaml documentIndex: 2 set: - database: - postgres: - bundled: - securityContext: - allowPrivilegeEscalation: false - readOnlyRootFilesystem: false - capabilities: - drop: - - ALL - seccompProfile: - type: Localhost - localhostProfile: profiles/postgres-container.json + substrate.enabled: true asserts: - equal: - path: spec.template.spec.containers[0].securityContext.allowPrivilegeEscalation - value: false - - equal: - path: spec.template.spec.containers[0].securityContext.readOnlyRootFilesystem - value: false - - contains: - path: spec.template.spec.containers[0].securityContext.capabilities.drop - content: ALL - - equal: - path: spec.template.spec.containers[0].securityContext.seccompProfile.type - value: Localhost + path: metadata.name + value: substrate-postgres-readwrite - equal: - path: spec.template.spec.containers[0].securityContext.seccompProfile.localhostProfile - value: profiles/postgres-container.json + path: stringData.readWriteConnectionString + value: postgresql://substrate_readwrite_user:substrate-readwrite@RELEASE-NAME-postgresql.NAMESPACE.svc:5432/kagent?sslmode=disable - - it: should render Service with hardcoded ClusterIP type and port 5432 - template: postgresql.yaml + - it: should create the Substrate owner Secret + template: postgresql-secret.yaml documentIndex: 3 + set: + substrate.enabled: true asserts: - - isKind: - of: Service - - equal: - path: spec.type - value: ClusterIP - equal: - path: spec.ports[0].port - value: 5432 - - - it: should use correct selector labels on Service - template: postgresql.yaml - documentIndex: 3 - asserts: + path: metadata.name + value: substrate-postgres-owner - equal: - path: spec.selector["app.kubernetes.io/component"] - value: database + path: stringData.ownerConnectionString + value: postgresql://substrate_admin_user:substrate-admin@RELEASE-NAME-postgresql.NAMESPACE.svc:5432/kagent?sslmode=disable - - it: should set serviceAccountName on Deployment - template: postgresql.yaml + - it: should pass the bundled database address to Substrate with a custom release name + template: postgresql-secret.yaml documentIndex: 2 + set: + fullnameOverride: custom-kagent + substrate.enabled: true asserts: - equal: - path: spec.template.spec.serviceAccountName - value: RELEASE-NAME-postgresql + path: stringData.readWriteConnectionString + value: postgresql://substrate_readwrite_user:substrate-readwrite@custom-kagent-postgresql.NAMESPACE.svc:5432/kagent?sslmode=disable - - it: should create ServiceAccount when bundled is enabled - template: postgresql.yaml - documentIndex: 0 + - it: should reject a different Substrate database for shared bundled PostgreSQL + template: postgresql-secret.yaml + set: + substrate.enabled: true + substrate.postgres.database: shared-db asserts: - - isKind: - of: ServiceAccount - - equal: - path: metadata.name - value: RELEASE-NAME-postgresql + - failedTemplate: + errorMessage: substrate.postgres.database must be kagent when sharing Kagent's bundled PostgreSQL - # ============================================================================= - # postgresql-secret.yaml - # ============================================================================= - - - it: should create secret in bundled mode + - it: should reject mismatched administrator Secret references for shared bundled PostgreSQL template: postgresql-secret.yaml + set: + substrate.enabled: true + database.postgres.bundled.adminSecretRef.name: custom-admin asserts: - - isKind: - of: Secret - - equal: - path: metadata.name - value: RELEASE-NAME-postgresql - - isNotNull: - path: data.POSTGRES_PASSWORD + - failedTemplate: + errorMessage: substrate.postgres.adminSecretRef must match database.postgres.bundled.adminSecretRef when sharing bundled PostgreSQL - - it: should base64-encode the hardcoded demo password + - it: should accept matching custom administrator Secret references template: postgresql-secret.yaml + documentIndex: 1 + set: + substrate.enabled: true + database.postgres.bundled.adminSecretRef.name: custom-admin + substrate.postgres.adminSecretRef.name: custom-admin asserts: - equal: - path: data.POSTGRES_PASSWORD - # echo -n "kagent" | base64 - value: "a2FnZW50" + path: metadata.name + value: substrate-postgres-readwrite - - it: should not create secret when bundled is disabled + - it: should reject operator-managed Substrate Secrets during bootstrap template: postgresql-secret.yaml set: - database: - postgres: - bundled: - enabled: false + substrate.enabled: true + substrate.postgres.ownerConnectionStringSecretRef.name: custom-owner asserts: - - hasDocuments: - count: 0 + - failedTemplate: + errorMessage: substrate.postgres.bootstrap requires the chart-managed Substrate application Secret names; disable bootstrap for operator-managed Secrets - - it: should still create secret when url is set and bundled is enabled + - it: should not create Secrets for an external managed database template: postgresql-secret.yaml set: - database: - postgres: - url: "postgres://user:pass@host:5432/db" + database.postgres: + bundled: + enabled: false + secretRef: + name: kagent-database asserts: - hasDocuments: - count: 1 + count: 0 - - it: should still create secret when urlFile is set and bundled is enabled + - it: should keep operator Secrets unmanaged template: postgresql-secret.yaml set: - database: - postgres: - urlFile: "/var/secrets/db-url" + database.postgres.bundled.bootstrap: false + database.postgres.bundled.adminSecretRef.name: custom-admin + database.postgres.secretRef.name: custom-kagent asserts: - hasDocuments: - count: 1 + count: 0 - - it: should not render imagePullSecret by default + - it: should apply the database pod labels template: postgresql.yaml documentIndex: 2 + set: + podLabels: + team: platform + database.postgres.bundled.podLabels: + tier: data asserts: - - isKind: - of: Deployment - - notExists: - path: spec.template.spec.imagePullSecrets + - equal: + path: spec.template.metadata.labels.team + value: platform + - equal: + path: spec.template.metadata.labels.tier + value: data - - it: should render imagePullSecret when available + - it: should apply image, resources, security, and scheduling settings template: postgresql.yaml documentIndex: 2 set: global.imagePullSecrets: - - name: secret1 - - name: secret2 + - name: registry + database.postgres.bundled: + image: + registry: registry.example.com + repository: platform + name: postgres + tag: "18" + resources: + requests: + cpu: 500m + memory: 512Mi + limits: + cpu: "2" + memory: 2Gi + podSecurityContext: + runAsNonRoot: true + runAsUser: 1000 + securityContext: + allowPrivilegeEscalation: false + nodeSelector: + role: database + tolerations: + - key: dedicated + operator: Equal + value: database + effect: NoSchedule + affinity: + nodeAffinity: + requiredDuringSchedulingIgnoredDuringExecution: + nodeSelectorTerms: + - matchExpressions: + - key: topology.kubernetes.io/zone + operator: Exists asserts: - - isKind: - of: Deployment - equal: - path: spec.template.spec.imagePullSecrets - value: - - name: secret1 - - name: secret2 - - # ============================================================================= - # scheduling (bundled PostgreSQL pod) - # ============================================================================= + path: spec.template.spec.containers[0].image + value: registry.example.com/platform/postgres:18 + - equal: + path: spec.template.spec.containers[0].resources.limits.memory + value: 2Gi + - equal: + path: spec.template.spec.securityContext.runAsUser + value: 1000 + - equal: + path: spec.template.spec.containers[0].securityContext.allowPrivilegeEscalation + value: false + - equal: + path: spec.template.spec.imagePullSecrets[0].name + value: registry + - equal: + path: spec.template.spec.nodeSelector.role + value: database + - equal: + path: spec.template.spec.tolerations[0].effect + value: NoSchedule + - equal: + path: spec.template.spec.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms[0].matchExpressions[0].key + value: topology.kubernetes.io/zone - - it: should not set scheduling fields by default + - it: should keep the default storage configuration template: postgresql.yaml - documentIndex: 2 + documentIndex: 1 asserts: - - isKind: - of: Deployment - - notExists: - path: spec.template.spec.nodeSelector - - notExists: - path: spec.template.spec.tolerations + - equal: + path: spec.resources.requests.storage + value: 500Mi - notExists: - path: spec.template.spec.affinity + path: spec.storageClassName - - it: should set nodeSelector + - it: should omit an empty image repository template: postgresql.yaml documentIndex: 2 set: - database: - postgres: - bundled: - nodeSelector: - role: AI + database.postgres.bundled.image: + registry: docker.io + repository: "" + name: postgres + tag: "18" asserts: - equal: - path: spec.template.spec.nodeSelector - value: - role: AI + path: spec.template.spec.containers[0].image + value: docker.io/postgres:18 - - it: should set tolerations + - it: should keep database paths and default resources template: postgresql.yaml documentIndex: 2 - set: - database: - postgres: - bundled: - tolerations: - - key: dedicated - operator: Equal - value: ai - effect: NoSchedule asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: PGDATA + value: /var/lib/postgresql/data/pgdata - equal: - path: spec.template.spec.tolerations - value: - - key: dedicated - operator: Equal - value: ai - effect: NoSchedule + path: spec.template.spec.containers[0].resources.requests.cpu + value: 250m + - equal: + path: spec.template.spec.containers[0].resources.limits.memory + value: 512Mi - - it: should set affinity + - it: should keep database security defaults template: postgresql.yaml documentIndex: 2 - set: - database: - postgres: - bundled: - affinity: - nodeAffinity: - requiredDuringSchedulingIgnoredDuringExecution: - nodeSelectorTerms: - - matchExpressions: - - key: topology.kubernetes.io/zone - operator: In - values: - - eu-west-1a asserts: - equal: - path: spec.template.spec.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms[0].matchExpressions[0].key - value: topology.kubernetes.io/zone + path: spec.template.spec.securityContext.runAsNonRoot + value: true + - equal: + path: spec.template.spec.securityContext.runAsUser + value: 999 + - equal: + path: spec.template.spec.containers[0].securityContext.allowPrivilegeEscalation + value: false + + - it: should configure the database Service + template: postgresql.yaml + documentIndex: 3 + asserts: + - equal: + path: spec.type + value: ClusterIP + - equal: + path: spec.ports[0].port + value: 5432 + - equal: + path: spec.selector["app.kubernetes.io/component"] + value: database + + - it: should use the database ServiceAccount + template: postgresql.yaml + documentIndex: 2 + asserts: + - equal: + path: spec.template.spec.serviceAccountName + value: RELEASE-NAME-postgresql + + - it: should omit optional pod fields by default + template: postgresql.yaml + documentIndex: 2 + asserts: + - notExists: + path: spec.template.spec.imagePullSecrets + - notExists: + path: spec.template.spec.nodeSelector + - notExists: + path: spec.template.spec.tolerations + - notExists: + path: spec.template.spec.affinity diff --git a/helm/kagent/values.yaml b/helm/kagent/values.yaml index 83de7e509f..977bf5b3fc 100644 --- a/helm/kagent/values.yaml +++ b/helm/kagent/values.yaml @@ -104,12 +104,23 @@ nodeSelector: {} database: postgres: - # -- External PostgreSQL connection string. - # Is always used if set regardless of the `.bundled.enabled` field. - url: "" - # -- Path to a file containing the database URL. Takes precedence over url when set. - # Is always used if set regardless of the `.bundled.enabled` field. - urlFile: "" + # -- Source the PostgreSQL connection string from an existing Secret when + # bundled bootstrap is disabled or PostgreSQL is external. + # The Secret is mounted and reread for each new physical connection. Configure + # pool.maxConnLifetime to bound how long old credentials remain in use. + # Embedded Substrate uses separate read/write and owner connection Secrets + # even when it shares the same database. + secretRef: + name: "" + key: connectionString + # -- Role assumed on each Kagent connection. Set a distinct role for each + # install sharing a database; bundled bootstrap requires kagent_owner. + role: kagent_owner + # -- Schema for Kagent tables. + schema: kagent + # -- Schema for the shared pgvector extension. Existing installations must match this value. + # Kagent needs USAGE on this schema; only trusted administrators should have CREATE on it. + vectorSchema: extensions # -- Enable the pgvector migration # Required to use features that depend on database vector capability. (e.g. long-term memory) # Set to true when using an external PostgreSQL that has the pgvector extension installed. @@ -124,12 +135,22 @@ database: maxConns: null minConns: null maxConnIdleTime: "" + # -- Maximum physical connection lifetime. This bounds Secret credential turnover. maxConnLifetime: "" # -- Bundled PostgreSQL instance — for development and evaluation only. - # Not suitable for production. Deployed when enabled is true and url/urlFile are not set. + # Not suitable for production. Deployed whenever enabled is true. bundled: - # -- Set to false to disable the bundled database and provide your own via url or urlFile. + # -- Set to false to disable the bundled database and provide an external connection. enabled: true + # -- Create the fixed Kagent identity and schema. Disable when providing + # existing users and a connection Secret for the bundled database. + bootstrap: true + adminSecretRef: + # -- Existing administrator Secret. The chart creates postgres-admin when this value is empty. + name: "" + usernameKey: POSTGRES_USER + passwordKey: POSTGRES_PASSWORD + image: # -- Bundled PostgreSQL image registry registry: docker.io @@ -147,8 +168,7 @@ database: storage: 500Mi # -- StorageClass for the PostgreSQL PVC. Defaults to the cluster default when empty. storageClassName: "" - # The database name, user, and password are hardcoded for the bundled instance (all: "kagent"). - # This is intentional for a dev/eval setup. Switch to an external database for production. + # The bundled database uses fixed development credentials. # -- Resource requests/limits for the demo PostgreSQL container resources: requests: @@ -758,6 +778,39 @@ kmcp: substrate: enabled: false + postgres: + # Kagent and Substrate use separate schemas in the same database. + enabled: false + schema: substrate + # -- Read the Substrate read/write connection string from a Secret. + # With bundled bootstrap enabled, Kagent renders the fixed Substrate + # credentials from the Substrate chart into this Secret. + readWriteConnectionStringSecretRef: + # -- Use this fixed name for bundled bootstrap; otherwise supply an existing Secret. + name: substrate-postgres-readwrite + key: readWriteConnectionString + # -- Read the owner connection string from a Secret. + ownerConnectionStringSecretRef: + name: substrate-postgres-owner + key: ownerConnectionString + # -- Kagent's bundled PostgreSQL creates only the kagent database. + database: kagent + bootstrap: true + # -- Roles assumed by Substrate. Set distinct roles for each BYO install; + # Substrate bootstrap requires the fixed defaults. + readWriteRole: substrate_readwrite + ownerRole: substrate_owner + adminSecretRef: + name: postgres-admin + usernameKey: POSTGRES_USER + passwordKey: POSTGRES_PASSWORD + pool: + # -- Maximum physical connection lifetime for Substrate's pools. + # Bounds how long a rotated credential stays in use. Set it longer than the + # delay before the platform publishes an updated Secret, or connections + # retire before the new credential arrives. Empty keeps the pgx default, + # which retires connections after one hour. + maxConnLifetime: "" # Grant each agent atespace access to its credential namespace explicitly. # HTTPS also requires the egress-mitm-ca-pool Secret in Substrate's namespace. credentialProvider: