From 7af80e8dee8bf43f3c9ca75fd6a42accb3c0898d Mon Sep 17 00:00:00 2001 From: Jeremy Alvis Date: Fri, 4 Sep 2026 11:39:14 -0700 Subject: [PATCH 01/12] Share PostgreSQL with Substrate Configure embedded Substrate to use Kagent's selected PostgreSQL database through a shared Secret and a separate schema. Support bundled, external, shared existing, and independently configured Substrate databases, with examples for each installation mode. Signed-off-by: Jeremy Alvis --- helm/README.md | 38 +++++++++++ .../templates/controller-deployment.yaml | 10 ++- helm/kagent/templates/postgresql-secret.yaml | 26 ++++++++ .../tests/controller-deployment_test.yaml | 65 +++++++++++++++++++ helm/kagent/tests/postgresql_test.yaml | 59 +++++++++++++++++ helm/kagent/values.yaml | 16 ++++- 6 files changed, 211 insertions(+), 3 deletions(-) diff --git a/helm/README.md b/helm/README.md index 8f924b7716..ab782dcc87 100644 --- a/helm/README.md +++ b/helm/README.md @@ -21,6 +21,44 @@ 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 + +Enabling Substrate uses Kagent's bundled PostgreSQL by default. Kagent and +Substrate share the database connection but use separate schemas. + +```yaml +substrate: + enabled: true +``` + +To share an external PostgreSQL connection, configure it once for Kagent: + +```yaml +database: + postgres: + url: postgresql://user:password@database:5432/kagent + bundled: + enabled: false +substrate: + enabled: true +``` + +To give Substrate a separate PostgreSQL connection, disable sharing and set +the Substrate connection directly: + +```yaml +substrate: + enabled: true + postgres: + connectionString: postgresql://user:password@substrate-db:5432/substrate + connectionStringSecretRef: + enabled: false +``` + +For a separate Secret-backed connection, leave `enabled: false` and set +`connectionStringSecretRef.name` and `key`. A pod-local +`database.postgres.urlFile` cannot be shared with Substrate. + ### Using Make ```bash diff --git a/helm/kagent/templates/controller-deployment.yaml b/helm/kagent/templates/controller-deployment.yaml index 6c8d8cacf9..cd707d3264 100644 --- a/helm/kagent/templates/controller-deployment.yaml +++ b/helm/kagent/templates/controller-deployment.yaml @@ -101,7 +101,15 @@ spec: - name: AUTH_USER_ID_CLAIM value: {{ .Values.controller.auth.userIdClaim | quote }} {{- end }} - {{- if .Values.database.postgres.urlFile }} + {{- $substratePostgres := get .Values.substrate "postgres" | default dict }} + {{- $connectionStringSecretRef := get $substratePostgres "connectionStringSecretRef" | default dict }} + {{- if and .Values.substrate.enabled (get $connectionStringSecretRef "enabled") }} + - name: POSTGRES_DATABASE_URL + valueFrom: + secretKeyRef: + name: {{ get $connectionStringSecretRef "name" | default (include "substrate.fullname" (list "postgres-connection" .)) }} + key: {{ get $connectionStringSecretRef "key" | default "connectionString" }} + {{- else if .Values.database.postgres.urlFile }} - name: POSTGRES_DATABASE_URL_FILE value: {{ .Values.database.postgres.urlFile | quote }} {{- else if .Values.database.postgres.url }} diff --git a/helm/kagent/templates/postgresql-secret.yaml b/helm/kagent/templates/postgresql-secret.yaml index 3adb5b3c4e..371f06c125 100644 --- a/helm/kagent/templates/postgresql-secret.yaml +++ b/helm/kagent/templates/postgresql-secret.yaml @@ -11,3 +11,29 @@ type: Opaque data: POSTGRES_PASSWORD: {{ "kagent" | b64enc | quote }} {{- end }} +{{- $substratePostgres := get .Values.substrate "postgres" | default dict -}} +{{- $connectionStringSecretRef := get $substratePostgres "connectionStringSecretRef" | default dict -}} +{{- if and .Values.substrate.enabled (get $connectionStringSecretRef "enabled") (not (get $connectionStringSecretRef "name")) }} +{{- $connectionString := "" -}} +{{- if .Values.database.postgres.urlFile -}} +{{- fail "database.postgres.urlFile cannot configure embedded Substrate; set substrate.postgres.connectionStringSecretRef.name to the Secret containing the URL" -}} +{{- else if .Values.database.postgres.url -}} +{{- $connectionString = .Values.database.postgres.url -}} +{{- else if .Values.database.postgres.bundled.enabled -}} +{{- $connectionString = printf "postgres://kagent:kagent@%s.%s.svc:5432/kagent?sslmode=disable" (include "kagent.postgresqlServiceName" .) (include "kagent.namespace" .) -}} +{{- else -}} +{{- fail "No database connection configured. Set database.postgres.url, substrate.postgres.connectionStringSecretRef.name, or enable database.postgres.bundled." -}} +{{- end }} +--- +apiVersion: v1 +kind: Secret +metadata: + name: {{ get $connectionStringSecretRef "name" | default (include "substrate.fullname" (list "postgres-connection" .)) }} + namespace: {{ include "kagent.namespace" . }} + labels: + {{- include "kagent.labels" . | nindent 4 }} + app.kubernetes.io/component: database +type: Opaque +stringData: + {{ get $connectionStringSecretRef "key" | default "connectionString" }}: {{ $connectionString | quote }} +{{- end }} diff --git a/helm/kagent/tests/controller-deployment_test.yaml b/helm/kagent/tests/controller-deployment_test.yaml index 5471b11292..b8056be9a2 100644 --- a/helm/kagent/tests/controller-deployment_test.yaml +++ b/helm/kagent/tests/controller-deployment_test.yaml @@ -448,6 +448,71 @@ tests: name: POSTGRES_DATABASE_URL value: "postgres://user:pass@external-host:5432/db" + - it: should read the bundled database URL from the shared Substrate secret + template: controller-deployment.yaml + set: + substrate: + enabled: true + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: POSTGRES_DATABASE_URL + valueFrom: + secretKeyRef: + name: RELEASE-NAME-postgres-connection + key: connectionString + - notContains: + path: spec.template.spec.containers[0].env + content: + name: POSTGRES_PASSWORD + + - it: should read an existing shared database secret with Substrate + template: controller-deployment.yaml + set: + substrate: + enabled: true + postgres: + connectionStringSecretRef: + name: shared-db + key: url + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: POSTGRES_DATABASE_URL + valueFrom: + secretKeyRef: + name: shared-db + key: url + + - it: should keep an explicitly separate Substrate database Secret separate + template: controller-deployment.yaml + set: + substrate: + enabled: true + postgres: + connectionStringSecretRef: + enabled: false + name: substrate-db + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: POSTGRES_PASSWORD + valueFrom: + secretKeyRef: + name: RELEASE-NAME-postgresql + key: POSTGRES_PASSWORD + - 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 template: controller-deployment.yaml set: diff --git a/helm/kagent/tests/postgresql_test.yaml b/helm/kagent/tests/postgresql_test.yaml index 5d6d40a018..16ed8fc572 100644 --- a/helm/kagent/tests/postgresql_test.yaml +++ b/helm/kagent/tests/postgresql_test.yaml @@ -419,6 +419,65 @@ tests: - hasDocuments: count: 1 + - it: should create a shared Substrate connection secret for bundled postgres + template: postgresql-secret.yaml + documentIndex: 1 + set: + substrate: + enabled: true + asserts: + - isKind: + of: Secret + - equal: + path: metadata.name + value: RELEASE-NAME-postgres-connection + - equal: + path: stringData.connectionString + value: "postgres://kagent:kagent@RELEASE-NAME-postgresql.NAMESPACE.svc:5432/kagent?sslmode=disable" + + - it: should put an external URL in the shared Substrate connection secret + template: postgresql-secret.yaml + set: + database: + postgres: + url: "postgres://user:pass@external-host:5432/db" + bundled: + enabled: false + substrate: + enabled: true + asserts: + - equal: + path: stringData.connectionString + value: "postgres://user:pass@external-host:5432/db" + + - it: should use an existing Substrate connection secret without creating one + template: postgresql-secret.yaml + set: + database: + postgres: + bundled: + enabled: false + substrate: + enabled: true + postgres: + connectionStringSecretRef: + name: shared-db + asserts: + - hasDocuments: + count: 0 + + - it: should reject urlFile when a managed Substrate connection secret is required + template: postgresql-secret.yaml + set: + database: + postgres: + urlFile: /var/secrets/db-url + substrate: + enabled: true + asserts: + - failedTemplate: + errorMessage: "database.postgres.urlFile cannot configure embedded Substrate; set substrate.postgres.connectionStringSecretRef.name to the Secret containing the URL" + - it: should not render imagePullSecret by default template: postgresql.yaml documentIndex: 2 diff --git a/helm/kagent/values.yaml b/helm/kagent/values.yaml index 775faeafe0..29b4b8cf4c 100644 --- a/helm/kagent/values.yaml +++ b/helm/kagent/values.yaml @@ -75,10 +75,12 @@ nodeSelector: {} database: postgres: # -- External PostgreSQL connection string. - # Is always used if set regardless of the `.bundled.enabled` field. + # Used ahead of `.bundled` unless an existing shared Substrate Secret is set. 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. + # Takes precedence over `.bundled` unless an existing shared Substrate Secret is set. + # Embedded Substrate cannot inherit an arbitrary mounted file; configure its + # connectionStringSecretRef.name instead. urlFile: "" # -- Enable the pgvector migration # Required to use features that depend on database vector capability. (e.g. long-term memory) @@ -664,6 +666,16 @@ kmcp: substrate: enabled: false + postgres: + # Kagent and Substrate use separate schemas in the same database. + enabled: false + schema: substrate + # -- Share Kagent's selected database with Substrate through a Secret. + connectionStringSecretRef: + enabled: true + # -- Existing Secret name. Kagent creates a release-named Secret when empty. + name: "" + key: connectionString # ============================================================================== # BUILT-IN TOOLS From 0d993e8a50a570ec7e3114bfa08d67afcee2e963 Mon Sep 17 00:00:00 2001 From: Jeremy Alvis Date: Fri, 4 Sep 2026 13:23:11 -0700 Subject: [PATCH 02/12] Replace PostgreSQL URL files with Secret references Signed-off-by: Jeremy Alvis --- contrib/cncf/technical-review.md | 2 +- go/core/internal/database/connect.go | 29 --------- go/core/internal/database/connect_test.go | 53 --------------- go/core/pkg/app/app.go | 5 +- helm/README.md | 22 ++++++- helm/kagent/templates/NOTES.txt | 10 +-- .../templates/controller-deployment.yaml | 20 +++--- helm/kagent/templates/postgresql-secret.yaml | 5 +- .../tests/controller-deployment_test.yaml | 64 +++++++++++-------- helm/kagent/tests/postgresql_test.yaml | 21 +++--- helm/kagent/values.yaml | 20 +++--- 11 files changed, 103 insertions(+), 148 deletions(-) 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/internal/database/connect.go b/go/core/internal/database/connect.go index 47daec9170..49d1730581 100644 --- a/go/core/internal/database/connect.go +++ b/go/core/internal/database/connect.go @@ -4,8 +4,6 @@ import ( "context" "fmt" "log" - "os" - "strings" "time" "github.com/jackc/pgx/v5" @@ -14,9 +12,6 @@ import ( ) // 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. -// // Pool fields are optional: nil leaves the corresponding pgxpool.Config value // from ParseConfig unchanged (pgx library defaults). type PostgresConfig struct { @@ -109,27 +104,3 @@ func retryDBConnection(ctx context.Context, cfg *PostgresConfig) (*pgxpool.Pool, } } } - -// ResolveURL returns url, unless urlFile is non-empty in which case the URL is -// read from that file. Used by callers (e.g. the migration runner) that need -// the resolved connection string before a pool is created. -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..c255704bda 100644 --- a/go/core/internal/database/connect_test.go +++ b/go/core/internal/database/connect_test.go @@ -2,8 +2,6 @@ package database import ( "context" - "os" - "path/filepath" "testing" "time" @@ -53,54 +51,3 @@ func TestApplyPoolConfig(t *testing.T) { assert.Equal(t, 10*time.Minute, config.MaxConnLifetime) }) } - -func TestResolveURLFile(t *testing.T) { - tests := []struct { - name string - fileContent string - wantUrl string - wantErr bool - }{ - { - 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, - }, - } - - 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) - }) - } - - t.Run("missing file returns error", func(t *testing.T) { - _, err := resolveURLFile("/nonexistent/path/db-url") - assert.Error(t, err) - }) -} diff --git a/go/core/pkg/app/app.go b/go/core/pkg/app/app.go index f2e23e411b..18b45830a7 100644 --- a/go/core/pkg/app/app.go +++ b/go/core/pkg/app/app.go @@ -163,10 +163,7 @@ 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")) - if err != nil { - return err - } + dbURL := env("POSTGRES_DATABASE_URL", "postgres://postgres:kagent@kagent-postgresql.kagent.svc.cluster.local:5432/postgres") vectorEnabled := kagentenv.DatabaseVectorEnabled.Get() // Appended, not merged: the built-in tracks must reach their final version // before a library consumer's tables, which may reference them. diff --git a/helm/README.md b/helm/README.md index ab782dcc87..30fb55e8ca 100644 --- a/helm/README.md +++ b/helm/README.md @@ -43,6 +43,25 @@ substrate: enabled: true ``` +To share an existing Secret, configure both charts to reference the same +name and key: + +```yaml +database: + postgres: + secretRef: + name: shared-postgres + key: connectionString + bundled: + enabled: false +substrate: + enabled: true + postgres: + connectionStringSecretRef: + name: shared-postgres + key: connectionString +``` + To give Substrate a separate PostgreSQL connection, disable sharing and set the Substrate connection directly: @@ -56,8 +75,7 @@ substrate: ``` For a separate Secret-backed connection, leave `enabled: false` and set -`connectionStringSecretRef.name` and `key`. A pod-local -`database.postgres.urlFile` cannot be shared with Substrate. +`connectionStringSecretRef.name` and `key`. ### Using Make diff --git a/helm/kagent/templates/NOTES.txt b/helm/kagent/templates/NOTES.txt index 12d573a010..4b40741104 100644 --- a/helm/kagent/templates/NOTES.txt +++ b/helm/kagent/templates/NOTES.txt @@ -64,7 +64,7 @@ DOCUMENTATION: {{- end }} {{ if .Values.database.postgres.bundled.enabled -}} ################################################################################ -{{- if and (eq .Values.database.postgres.url "") (eq .Values.database.postgres.urlFile "") }} +{{- if and (eq .Values.database.postgres.url "") (not .Values.database.postgres.secretRef.name) }} # WARNING: BUNDLED DATABASE IN USE # ################################################################################ The bundled PostgreSQL instance is enabled. It is intended for development and @@ -72,15 +72,17 @@ DOCUMENTATION: pod is restarted or rescheduled. To use an external database, set: - database.postgres.url= or database.postgres.urlFile= + database.postgres.url= + or database.postgres.secretRef.name= {{- 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). + external database. - To connect the controller to the bundled instance instead, unset url/urlFile: + To connect the controller to the bundled instance instead, unset the external connection: database.postgres.url="" + database.postgres.secretRef.name="" To stop deploying the bundled pod entirely, set: database.postgres.bundled.enabled=false {{- end }} diff --git a/helm/kagent/templates/controller-deployment.yaml b/helm/kagent/templates/controller-deployment.yaml index cd707d3264..09a6f916c1 100644 --- a/helm/kagent/templates/controller-deployment.yaml +++ b/helm/kagent/templates/controller-deployment.yaml @@ -1,3 +1,10 @@ +{{- $databaseConnectionStringSecretRef := .Values.database.postgres.secretRef | default dict -}} +{{- if hasKey .Values.database.postgres "urlFile" -}} +{{- fail "database.postgres.urlFile has been removed; use database.postgres.secretRef.{name,key}" -}} +{{- end -}} +{{- if and .Values.database.postgres.url (get $databaseConnectionStringSecretRef "name") -}} +{{- fail "database.postgres.url and database.postgres.secretRef.name are mutually exclusive" -}} +{{- end -}} apiVersion: apps/v1 kind: Deployment metadata: @@ -101,17 +108,12 @@ spec: - name: AUTH_USER_ID_CLAIM value: {{ .Values.controller.auth.userIdClaim | quote }} {{- end }} - {{- $substratePostgres := get .Values.substrate "postgres" | default dict }} - {{- $connectionStringSecretRef := get $substratePostgres "connectionStringSecretRef" | default dict }} - {{- if and .Values.substrate.enabled (get $connectionStringSecretRef "enabled") }} + {{- if get $databaseConnectionStringSecretRef "name" }} - name: POSTGRES_DATABASE_URL valueFrom: secretKeyRef: - name: {{ get $connectionStringSecretRef "name" | default (include "substrate.fullname" (list "postgres-connection" .)) }} - key: {{ get $connectionStringSecretRef "key" | default "connectionString" }} - {{- else if .Values.database.postgres.urlFile }} - - name: POSTGRES_DATABASE_URL_FILE - value: {{ .Values.database.postgres.urlFile | quote }} + name: {{ get $databaseConnectionStringSecretRef "name" }} + key: {{ get $databaseConnectionStringSecretRef "key" | default "connectionString" }} {{- else if .Values.database.postgres.url }} - name: POSTGRES_DATABASE_URL value: {{ .Values.database.postgres.url | quote }} @@ -124,7 +126,7 @@ spec: - 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." }} + {{ fail "No database connection configured. Set database.postgres.url, database.postgres.secretRef.name, or enable database.postgres.bundled." }} {{- end }} {{- if include "kagent.controller.metricsEnabled" . }} - name: METRICS_BIND_ADDRESS diff --git a/helm/kagent/templates/postgresql-secret.yaml b/helm/kagent/templates/postgresql-secret.yaml index 371f06c125..a8f7ee7e1f 100644 --- a/helm/kagent/templates/postgresql-secret.yaml +++ b/helm/kagent/templates/postgresql-secret.yaml @@ -11,12 +11,13 @@ type: Opaque data: POSTGRES_PASSWORD: {{ "kagent" | b64enc | quote }} {{- end }} +{{- $databaseConnectionStringSecretRef := .Values.database.postgres.secretRef | default dict -}} {{- $substratePostgres := get .Values.substrate "postgres" | default dict -}} {{- $connectionStringSecretRef := get $substratePostgres "connectionStringSecretRef" | default dict -}} {{- if and .Values.substrate.enabled (get $connectionStringSecretRef "enabled") (not (get $connectionStringSecretRef "name")) }} {{- $connectionString := "" -}} -{{- if .Values.database.postgres.urlFile -}} -{{- fail "database.postgres.urlFile cannot configure embedded Substrate; set substrate.postgres.connectionStringSecretRef.name to the Secret containing the URL" -}} +{{- if get $databaseConnectionStringSecretRef "name" -}} +{{- fail "database.postgres.secretRef cannot be inherited by Substrate; set substrate.postgres.connectionStringSecretRef to the same Secret" -}} {{- else if .Values.database.postgres.url -}} {{- $connectionString = .Values.database.postgres.url -}} {{- else if .Values.database.postgres.bundled.enabled -}} diff --git a/helm/kagent/tests/controller-deployment_test.yaml b/helm/kagent/tests/controller-deployment_test.yaml index b8056be9a2..b588383fa0 100644 --- a/helm/kagent/tests/controller-deployment_test.yaml +++ b/helm/kagent/tests/controller-deployment_test.yaml @@ -448,7 +448,7 @@ tests: name: POSTGRES_DATABASE_URL value: "postgres://user:pass@external-host:5432/db" - - it: should read the bundled database URL from the shared Substrate secret + - it: should keep Kagent's bundled database configuration when Substrate is enabled template: controller-deployment.yaml set: substrate: @@ -457,19 +457,25 @@ tests: - contains: path: spec.template.spec.containers[0].env content: - name: POSTGRES_DATABASE_URL + name: POSTGRES_PASSWORD valueFrom: secretKeyRef: - name: RELEASE-NAME-postgres-connection - key: connectionString - - notContains: + name: RELEASE-NAME-postgresql + key: POSTGRES_PASSWORD + - contains: path: spec.template.spec.containers[0].env content: - name: POSTGRES_PASSWORD + name: POSTGRES_DATABASE_URL + value: "postgres://kagent:$(POSTGRES_PASSWORD)@RELEASE-NAME-postgresql.NAMESPACE.svc:5432/kagent?sslmode=disable" - it: should read an existing shared database secret with Substrate template: controller-deployment.yaml set: + database: + postgres: + secretRef: + name: shared-db + key: url substrate: enabled: true postgres: @@ -513,18 +519,23 @@ tests: 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" + 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 + valueFrom: + secretKeyRef: + name: external-postgres + key: url - notContains: path: spec.template.spec.containers[0].env content: @@ -618,28 +629,27 @@ tests: content: name: POSTGRES_PASSWORD - - it: should set POSTGRES_DATABASE_URL_FILE and omit POSTGRES_PASSWORD when urlFile and bundled are both enabled + - it: should reject the removed urlFile value template: controller-deployment.yaml set: database: postgres: - urlFile: "/var/secrets/db-url" - bundled: - enabled: true + urlFile: /var/secrets/db-url 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.urlFile has been removed; use database.postgres.secretRef.{name,key}" + + - it: should reject both an inline URL and Secret reference + template: controller-deployment.yaml + set: + database: + postgres: + url: postgres://user:pass@external-host:5432/db + secretRef: + name: external-postgres + asserts: + - failedTemplate: + errorMessage: "database.postgres.url and database.postgres.secretRef.name are mutually exclusive" - it: should set external POSTGRES_DATABASE_URL and omit POSTGRES_PASSWORD when url and bundled are both enabled template: controller-deployment.yaml diff --git a/helm/kagent/tests/postgresql_test.yaml b/helm/kagent/tests/postgresql_test.yaml index 16ed8fc572..cc70f66e59 100644 --- a/helm/kagent/tests/postgresql_test.yaml +++ b/helm/kagent/tests/postgresql_test.yaml @@ -4,7 +4,7 @@ templates: - postgresql-secret.yaml tests: # ============================================================================= - # bundled mode (default — url and urlFile both empty, bundled.enabled true) + # bundled mode (default — no external connection, bundled.enabled true) # ============================================================================= - it: should render ServiceAccount, PVC, Deployment, and Service when bundled is enabled @@ -34,12 +34,13 @@ tests: - hasDocuments: count: 4 - - it: should still render resources when urlFile is set and bundled is enabled + - it: should still render resources when an external Secret is set and bundled is enabled template: postgresql.yaml set: database: postgres: - urlFile: "/var/secrets/db-url" + secretRef: + name: external-postgres asserts: - hasDocuments: count: 4 @@ -409,12 +410,13 @@ tests: - hasDocuments: count: 1 - - it: should still create secret when urlFile is set and bundled is enabled + - it: should still create the bundled password secret when an external Secret is set template: postgresql-secret.yaml set: database: postgres: - urlFile: "/var/secrets/db-url" + secretRef: + name: external-postgres asserts: - hasDocuments: count: 1 @@ -466,17 +468,20 @@ tests: - hasDocuments: count: 0 - - it: should reject urlFile when a managed Substrate connection secret is required + - it: should require an explicit Substrate reference for an existing Kagent Secret template: postgresql-secret.yaml set: database: postgres: - urlFile: /var/secrets/db-url + secretRef: + name: shared-db + bundled: + enabled: false substrate: enabled: true asserts: - failedTemplate: - errorMessage: "database.postgres.urlFile cannot configure embedded Substrate; set substrate.postgres.connectionStringSecretRef.name to the Secret containing the URL" + errorMessage: "database.postgres.secretRef cannot be inherited by Substrate; set substrate.postgres.connectionStringSecretRef to the same Secret" - it: should not render imagePullSecret by default template: postgresql.yaml diff --git a/helm/kagent/values.yaml b/helm/kagent/values.yaml index 29b4b8cf4c..1acf5ad137 100644 --- a/helm/kagent/values.yaml +++ b/helm/kagent/values.yaml @@ -75,13 +75,14 @@ nodeSelector: {} database: postgres: # -- External PostgreSQL connection string. - # Used ahead of `.bundled` unless an existing shared Substrate Secret is set. + # Mutually exclusive with `secretRef.name`. url: "" - # -- Path to a file containing the database URL. Takes precedence over url when set. - # Takes precedence over `.bundled` unless an existing shared Substrate Secret is set. - # Embedded Substrate cannot inherit an arbitrary mounted file; configure its - # connectionStringSecretRef.name instead. - urlFile: "" + # -- Source the external PostgreSQL connection string from an existing Secret. + # To share it with embedded Substrate, set the same name and key under + # `substrate.postgres.connectionStringSecretRef`. + secretRef: + name: "" + key: connectionString # -- 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. @@ -98,9 +99,9 @@ database: maxConnIdleTime: "" 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 image: # -- Bundled PostgreSQL image registry @@ -670,7 +671,8 @@ substrate: # Kagent and Substrate use separate schemas in the same database. enabled: false schema: substrate - # -- Share Kagent's selected database with Substrate through a Secret. + # -- Read the Substrate connection string from a Secret. + # With no name, Kagent creates this Secret from its bundled or inline URL. connectionStringSecretRef: enabled: true # -- Existing Secret name. Kagent creates a release-named Secret when empty. From a2b2b3adc47eff3927de6b351046e6342fdb2f7e Mon Sep 17 00:00:00 2001 From: Jeremy Alvis Date: Thu, 17 Sep 2026 10:13:09 -0700 Subject: [PATCH 03/12] Document separating substrate connection for keeping DDL off of kagent's role Signed-off-by: Jeremy Alvis --- helm/README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/helm/README.md b/helm/README.md index 72303c67f7..a97e71c7af 100644 --- a/helm/README.md +++ b/helm/README.md @@ -77,6 +77,14 @@ substrate: For a separate Secret-backed connection, leave `enabled: false` and set `connectionStringSecretRef.name` and `key`. +The separate Substrate connection can point to the same PostgreSQL server and +database as Kagent while using a different database role. This keeps +Substrate's required DDL privileges off Kagent's runtime role. + +Substrate currently uses one role for migrations, runtime access, and runtime +outbox-partition DDL. It does not support separate DDL/migration and +DML/runtime identities. + ### Using Make ```bash From f379cbb436fad9f3683afc6d2e5d3b1e6c830fc9 Mon Sep 17 00:00:00 2001 From: Jeremy Alvis Date: Fri, 18 Sep 2026 14:36:08 -0700 Subject: [PATCH 04/12] Add in separate ddl/dml support or substrate Signed-off-by: Jeremy Alvis --- helm/README.md | 30 ++++++++++++++++++++------ helm/kagent/tests/postgresql_test.yaml | 20 +++++++++++++++++ helm/kagent/values.yaml | 12 ++++++++++- 3 files changed, 54 insertions(+), 8 deletions(-) diff --git a/helm/README.md b/helm/README.md index a97e71c7af..233c4e7840 100644 --- a/helm/README.md +++ b/helm/README.md @@ -24,7 +24,7 @@ helm install kagent ./helm/kagent/ --namespace kagent --set providers.default=az ### Substrate PostgreSQL Enabling Substrate uses Kagent's bundled PostgreSQL by default. Kagent and -Substrate share the database connection but use separate schemas. +Substrate share the database and connection but use separate schemas. ```yaml substrate: @@ -77,13 +77,29 @@ substrate: For a separate Secret-backed connection, leave `enabled: false` and set `connectionStringSecretRef.name` and `key`. -The separate Substrate connection can point to the same PostgreSQL server and -database as Kagent while using a different database role. This keeps -Substrate's required DDL privileges off Kagent's runtime role. +For least privilege, give Substrate separate runtime/DML and DDL roles. Both +connections can point to Kagent's PostgreSQL server and database; the +`substrate` schema keeps their objects separate from Kagent's: -Substrate currently uses one role for migrations, runtime access, and runtime -outbox-partition DDL. It does not support separate DDL/migration and -DML/runtime identities. +```yaml +substrate: + enabled: true + postgres: + schema: substrate + connectionStringSecretRef: + name: substrate-postgres + key: runtimeConnectionString + ddlConnectionStringSecretRef: + name: substrate-postgres + key: ddlConnectionString +``` + +The DDL role owns the Substrate schema and performs migrations and partition +maintenance. After migrations, Substrate gives the runtime role access to the +tables and sequences created by the DDL role. Without this step, the runtime +connection would fail with permission-denied errors. When both connections use +the same role, no grant is needed. Omitting the DDL connection preserves +single-connection operation. ### Using Make diff --git a/helm/kagent/tests/postgresql_test.yaml b/helm/kagent/tests/postgresql_test.yaml index cc70f66e59..c5659b4018 100644 --- a/helm/kagent/tests/postgresql_test.yaml +++ b/helm/kagent/tests/postgresql_test.yaml @@ -468,6 +468,26 @@ tests: - hasDocuments: count: 0 + - it: should leave operator-provided runtime and DDL secrets unmanaged + template: postgresql-secret.yaml + set: + database: + postgres: + bundled: + enabled: false + substrate: + enabled: true + postgres: + connectionStringSecretRef: + name: substrate-database + key: runtimeUrl + ddlConnectionStringSecretRef: + name: substrate-database + key: ddlUrl + asserts: + - hasDocuments: + count: 0 + - it: should require an explicit Substrate reference for an existing Kagent Secret template: postgresql-secret.yaml set: diff --git a/helm/kagent/values.yaml b/helm/kagent/values.yaml index ca1a346d54..ade869a608 100644 --- a/helm/kagent/values.yaml +++ b/helm/kagent/values.yaml @@ -727,13 +727,23 @@ substrate: # Kagent and Substrate use separate schemas in the same database. enabled: false schema: substrate - # -- Read the Substrate connection string from a Secret. + # -- Optional inline Substrate runtime/DML connection string. + # Disable connectionStringSecretRef when using it. + connectionString: "" + # -- Read the Substrate runtime/DML connection string from a Secret. # With no name, Kagent creates this Secret from its bundled or inline URL. connectionStringSecretRef: enabled: true # -- Existing Secret name. Kagent creates a release-named Secret when empty. name: "" key: connectionString + # -- Optional DDL and maintenance connection. Defaults to the runtime connection. + ddlConnectionString: "" + # -- Read the optional DDL connection string from a Secret. + ddlConnectionStringSecretRef: + enabled: false + name: "" + key: ddlConnectionString ateApi: extraArgs: # Minimum supported interval for discovering and reconciling actor templates. From 25ac6cc260b763f49b3993f6d076ab36dab7c7c0 Mon Sep 17 00:00:00 2001 From: Jeremy Alvis Date: Mon, 21 Sep 2026 10:56:55 -0700 Subject: [PATCH 05/12] Rotate shared PostgreSQL credentials Signed-off-by: Jeremy Alvis --- go/core/internal/database/connect.go | 127 ++++++++++++- go/core/internal/database/connect_test.go | 172 ++++++++++++++++++ go/core/pkg/app/app.go | 27 ++- go/core/pkg/app/app_test.go | 25 +++ go/core/pkg/env/kagent.go | 28 +++ helm/README.md | 99 ++++++---- .../templates/controller-deployment.yaml | 25 ++- .../tests/controller-deployment_test.yaml | 88 ++++++++- helm/kagent/values.yaml | 3 + 9 files changed, 538 insertions(+), 56 deletions(-) diff --git a/go/core/internal/database/connect.go b/go/core/internal/database/connect.go index ebd84be2d5..29bf8460f3 100644 --- a/go/core/internal/database/connect.go +++ b/go/core/internal/database/connect.go @@ -2,16 +2,24 @@ 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/pkg/logging" pgvectorpgx "github.com/pgvector/pgvector-go/pgx" ) // PostgresConfig holds the connection parameters for a Postgres database. +// 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). type PostgresConfig struct { @@ -27,8 +35,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. @@ -60,25 +71,125 @@ func applyPoolConfig(config *pgxpool.Config, cfg *PostgresConfig) error { return 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. -func retryDBConnection(ctx context.Context, cfg *PostgresConfig) (*pgxpool.Pool, error) { - ctx, cancel := context.WithTimeout(ctx, defaultMaxTimeout) - defer cancel() +// 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 +} - config, err := pgxpool.ParseConfig(cfg.URL) +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 nil, fmt.Errorf("failed to parse database URL: %w", err) + 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 } + + 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 { + connConfig.Password = refreshed.Password + } + connConfig.TLSConfig = refreshed.TLSConfig + connConfig.Fallbacks = refreshed.Fallbacks + return nil + } + } + if cfg.VectorEnabled { config.AfterConnect = func(ctx context.Context, conn *pgx.Conn) error { return pgvectorpgx.RegisterTypes(ctx, conn) } } + return config, nil +} + +func sameConnectionIdentity(a, b *pgx.ConnConfig) bool { + if a.Host != b.Host || a.Port != b.Port || a.Database != b.Database || a.User != b.User { + 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. +func retryDBConnection(ctx context.Context, cfg *PostgresConfig) (*pgxpool.Pool, error) { + ctx, cancel := context.WithTimeout(ctx, defaultMaxTimeout) + defer cancel() + + config, err := poolConfig(cfg) + if err != nil { + return nil, err + } pool, err := pgxpool.NewWithConfig(ctx, config) if err != nil { diff --git a/go/core/internal/database/connect_test.go b/go/core/internal/database/connect_test.go index c255704bda..4e37eccd42 100644 --- a/go/core/internal/database/connect_test.go +++ b/go/core/internal/database/connect_test.go @@ -2,6 +2,8 @@ package database import ( "context" + "os" + "path/filepath" "testing" "time" @@ -51,3 +53,173 @@ func TestApplyPoolConfig(t *testing.T) { assert.Equal(t, 10*time.Minute, config.MaxConnLifetime) }) } + +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}) + require.NoError(t, err) + require.NotNil(t, config.BeforeConnect) + assert.Equal(t, "password-a", config.ConnConfig.Password) + + connConfig := config.ConnConfig.Copy() + initialTLS := connConfig.TLSConfig + writeDatabaseURL(t, path, "postgres://user: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) + assert.Equal(t, "kagent", connConfig.RuntimeParams["application_name"]) + assert.NotSame(t, initialTLS, connConfig.TLSConfig) +} + +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 + url string + }{ + {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: "user", url: "host=primary,secondary port=5432,5433 user=changed password=password-b dbname=app sslmode=disable"}, + {name: "fallback", url: "host=primary,changed port=5432,5433 user=runtime password=password-b dbname=app sslmode=disable"}, + } + + 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("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, + 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 writeDatabaseURL(t *testing.T, path, url string) { + t.Helper() + require.NoError(t, os.WriteFile(path, []byte(url), 0o600)) +} diff --git a/go/core/pkg/app/app.go b/go/core/pkg/app/app.go index a6a4a67cee..5be1081eab 100644 --- a/go/core/pkg/app/app.go +++ b/go/core/pkg/app/app.go @@ -170,7 +170,11 @@ func Run(ctx context.Context, opts Options) error { } }() - dbURL := env("POSTGRES_DATABASE_URL", "postgres://postgres:kagent@kagent-postgresql.kagent.svc.cluster.local:5432/postgres") + 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 fmt.Errorf("resolve database connection: %w", err) + } vectorEnabled := kagentenv.DatabaseVectorEnabled.Get() // Appended, not merged: the built-in tracks must reach their final version // before a library consumer's tables, which may reference them. @@ -182,7 +186,7 @@ func Run(ctx context.Context, opts Options) error { } else if err := migrations.RunUp(ctx, dbURL, 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 } @@ -366,6 +370,25 @@ func envBool(name string) bool { return value } +func postgresConfigFromEnv(source string, vectorEnabled bool) *database.PostgresConfig { + config := &database.PostgresConfig{URL: source, 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 +} + func namespaces(value string) []string { var result []string for namespace := range strings.SplitSeq(value, ",") { diff --git a/go/core/pkg/app/app_test.go b/go/core/pkg/app/app_test.go index 697cddf9e0..e7aab2a0d9 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" @@ -120,6 +121,30 @@ 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") + + 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.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/env/kagent.go b/go/core/pkg/env/kagent.go index afa1222d79..45534881b0 100644 --- a/go/core/pkg/env/kagent.go +++ b/go/core/pkg/env/kagent.go @@ -119,4 +119,32 @@ var ( "Verify required database migrations at startup without applying them.", 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/helm/README.md b/helm/README.md index 233c4e7840..782bfa0dc9 100644 --- a/helm/README.md +++ b/helm/README.md @@ -23,83 +23,110 @@ helm install kagent ./helm/kagent/ --namespace kagent --set providers.default=az ### Substrate PostgreSQL -Enabling Substrate uses Kagent's bundled PostgreSQL by default. Kagent and -Substrate share the database and connection but use separate schemas. +Kagent supports three PostgreSQL layouts with embedded Substrate. + +1. Share Kagent's bundled PostgreSQL. Kagent and Substrate use the same + database with separate schemas; the parent chart creates Substrate's + release-scoped connection Secret. ```yaml substrate: enabled: true ``` -To share an external PostgreSQL connection, configure it once for Kagent: +2. Share one external Secret. Helm cannot dynamically copy a parent Secret + reference into a dependency, so repeat the same name and key explicitly. ```yaml database: postgres: - url: postgresql://user:password@database:5432/kagent + secretRef: + name: shared-postgres + key: connectionString bundled: enabled: false substrate: enabled: true + postgres: + enabled: false + connectionStringSecretRef: + enabled: true + name: shared-postgres + key: connectionString ``` -To share an existing Secret, configure both charts to reference the same -name and key: +3. Use separate Kagent, Substrate runtime/DML, and Substrate DDL/maintenance + Secrets. ```yaml database: postgres: secretRef: - name: shared-postgres + name: kagent-postgres key: connectionString bundled: enabled: false substrate: enabled: true postgres: + enabled: false + schema: substrate connectionStringSecretRef: - name: shared-postgres + enabled: true + name: substrate-runtime-postgres + key: connectionString + ddlConnectionStringSecretRef: + enabled: true + name: substrate-ddl-postgres key: connectionString ``` -To give Substrate a separate PostgreSQL connection, disable sharing and set -the Substrate connection directly: +The DDL role owns the Substrate schema and performs migrations and partition +maintenance. Substrate grants its runtime role access to migrated tables and +sequences. Omitting the DDL connection preserves single-connection operation. + +An inline `database.postgres.url` remains supported, but is fixed for the life +of the controller process. When embedded Substrate is enabled, the parent chart +can copy that inline value into its release-scoped Substrate Secret. + +#### Credential rotation + +`database.postgres.secretRef` is mounted through a Secret volume. Kagent +rereads the connection string before opening each new physical connection; +existing sessions remain valid until pgx retires them. Set +`database.postgres.pool.maxConnLifetime` to bound Kagent's turnover time. When +embedded Substrate shares the Secret, set +`substrate.postgres.pool.maxConnLifetime` as well to bound its runtime, watch, +and DDL pools. Keep old and new credentials valid long enough for Kubernetes +Secret projection and connection turnover. + +Rotation may change passwords and referenced TLS material. Host, port, +fallback targets, database, and username identify the pool and require a +controller restart when changed. Direct binary deployments use +`POSTGRES_DATABASE_URL=@file:/absolute/path`; there is no separate `_FILE` +environment variable. + +Kagent 1.x removes `database.postgres.urlFile`. Replace: ```yaml -substrate: - enabled: true +database: postgres: - connectionString: postgresql://user:password@substrate-db:5432/substrate - connectionStringSecretRef: - enabled: false + urlFile: /user-managed/path ``` -For a separate Secret-backed connection, leave `enabled: false` and set -`connectionStringSecretRef.name` and `key`. - -For least privilege, give Substrate separate runtime/DML and DDL roles. Both -connections can point to Kagent's PostgreSQL server and database; the -`substrate` schema keeps their objects separate from Kagent's: +with: ```yaml -substrate: - enabled: true +database: postgres: - schema: substrate - connectionStringSecretRef: - name: substrate-postgres - key: runtimeConnectionString - ddlConnectionStringSecretRef: - name: substrate-postgres - key: ddlConnectionString + secretRef: + name: postgres-connection + key: connectionString ``` -The DDL role owns the Substrate schema and performs migrations and partition -maintenance. After migrations, Substrate gives the runtime role access to the -tables and sequences created by the DDL role. Without this step, the runtime -connection would fail with permission-denied errors. When both connections use -the same role, no grant is needed. Omitting the DDL connection preserves -single-connection operation. +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 diff --git a/helm/kagent/templates/controller-deployment.yaml b/helm/kagent/templates/controller-deployment.yaml index a2dfaffa37..90e7537a26 100644 --- a/helm/kagent/templates/controller-deployment.yaml +++ b/helm/kagent/templates/controller-deployment.yaml @@ -1,4 +1,7 @@ {{- $databaseConnectionStringSecretRef := .Values.database.postgres.secretRef | default dict -}} +{{- $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 -}} @@ -45,8 +48,16 @@ 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) }} + {{- if or (get $databaseConnectionStringSecretRef "name") (gt (len .Values.controller.volumes) 0) (and .Values.controller.substrate .Values.controller.substrate.enabled) }} volumes: + {{- if get $databaseConnectionStringSecretRef "name" }} + - name: {{ $databaseSecretVolumeName }} + secret: + secretName: {{ get $databaseConnectionStringSecretRef "name" }} + items: + - key: {{ get $databaseConnectionStringSecretRef "key" | default "connectionString" }} + path: {{ $databaseSecretFileName }} + {{- end }} {{- if and .Values.controller.substrate .Values.controller.substrate.enabled }} - name: substrate-servicedns projected: @@ -110,10 +121,7 @@ spec: {{- end }} {{- if get $databaseConnectionStringSecretRef "name" }} - name: POSTGRES_DATABASE_URL - valueFrom: - secretKeyRef: - name: {{ get $databaseConnectionStringSecretRef "name" }} - key: {{ get $databaseConnectionStringSecretRef "key" | default "connectionString" }} + value: {{ printf "@file:%s/%s" $databaseSecretMountPath $databaseSecretFileName | quote }} {{- else if .Values.database.postgres.url }} - name: POSTGRES_DATABASE_URL value: {{ .Values.database.postgres.url | quote }} @@ -207,8 +215,13 @@ spec: port: http periodSeconds: 30 {{- end }} - {{- if or (gt (len .Values.controller.volumeMounts) 0) (and .Values.controller.substrate .Values.controller.substrate.enabled) }} + {{- if or (get $databaseConnectionStringSecretRef "name") (gt (len .Values.controller.volumeMounts) 0) (and .Values.controller.substrate .Values.controller.substrate.enabled) }} volumeMounts: + {{- if get $databaseConnectionStringSecretRef "name" }} + - name: {{ $databaseSecretVolumeName }} + mountPath: {{ $databaseSecretMountPath }} + readOnly: true + {{- end }} {{- if and .Values.controller.substrate .Values.controller.substrate.enabled }} - name: substrate-servicedns mountPath: /run/substrate-servicedns diff --git a/helm/kagent/tests/controller-deployment_test.yaml b/helm/kagent/tests/controller-deployment_test.yaml index c63b2ddb8a..edea5640ca 100644 --- a/helm/kagent/tests/controller-deployment_test.yaml +++ b/helm/kagent/tests/controller-deployment_test.yaml @@ -528,10 +528,24 @@ tests: path: spec.template.spec.containers[0].env content: name: POSTGRES_DATABASE_URL - valueFrom: - secretKeyRef: - name: shared-db - key: url + value: "@file:/var/run/secrets/kagent/postgres/connection-string" + - contains: + path: spec.template.spec.volumes + content: + name: postgres-connection + secret: + secretName: shared-db + 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 - it: should keep an explicitly separate Substrate database Secret separate template: controller-deployment.yaml @@ -570,6 +584,11 @@ tests: key: url asserts: - contains: + path: spec.template.spec.containers[0].env + content: + 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 @@ -577,11 +596,72 @@ tests: secretKeyRef: name: external-postgres key: url + - contains: + path: spec.template.spec.volumes + content: + name: postgres-connection + secret: + secretName: 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 + - it: should keep Kagent separate from Substrate runtime and DDL Secrets + template: controller-deployment.yaml + set: + database: + postgres: + bundled: + enabled: false + secretRef: + name: kagent-db + key: kagent-url + substrate: + enabled: true + postgres: + connectionStringSecretRef: + enabled: true + name: substrate-runtime-db + key: runtime-url + ddlConnectionStringSecretRef: + enabled: true + name: substrate-ddl-db + key: ddl-url + asserts: + - contains: + path: spec.template.spec.volumes + content: + name: postgres-connection + secret: + secretName: kagent-db + items: + - key: kagent-url + path: connection-string + - notContains: + path: spec.template.spec.volumes + content: + name: postgres-connection + secret: + secretName: substrate-runtime-db + - notContains: + path: spec.template.spec.volumes + content: + name: postgres-connection + secret: + secretName: substrate-ddl-db + - it: should set DATABASE_VECTOR_ENABLED to false by default template: controller-configmap.yaml asserts: diff --git a/helm/kagent/values.yaml b/helm/kagent/values.yaml index ade869a608..af532c5230 100644 --- a/helm/kagent/values.yaml +++ b/helm/kagent/values.yaml @@ -78,6 +78,8 @@ database: # Mutually exclusive with `secretRef.name`. url: "" # -- Source the external PostgreSQL connection string from an existing Secret. + # The Secret is mounted and reread for each new physical connection. Configure + # pool.maxConnLifetime to bound how long old credentials remain in use. # To share it with embedded Substrate, set the same name and key under # `substrate.postgres.connectionStringSecretRef`. secretRef: @@ -97,6 +99,7 @@ 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 whenever enabled is true. From 26856cb0bcb6363be00ed34576660d79fdf941e6 Mon Sep 17 00:00:00 2001 From: Jeremy Alvis Date: Tue, 22 Sep 2026 07:14:35 -0700 Subject: [PATCH 06/12] Adopt a rotated PostgreSQL user on new connections Signed-off-by: Jeremy Alvis --- go/core/internal/database/connect.go | 7 ++++++- go/core/internal/database/connect_test.go | 6 ++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/go/core/internal/database/connect.go b/go/core/internal/database/connect.go index 29bf8460f3..ab57a0c3b5 100644 --- a/go/core/internal/database/connect.go +++ b/go/core/internal/database/connect.go @@ -145,6 +145,11 @@ func poolConfig(cfg *PostgresConfig) (*pgxpool.Config, error) { refreshed := fresh.ConnConfig.Config.Copy() if fileBacked { + // 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 @@ -162,7 +167,7 @@ func poolConfig(cfg *PostgresConfig) (*pgxpool.Config, error) { } func sameConnectionIdentity(a, b *pgx.ConnConfig) bool { - if a.Host != b.Host || a.Port != b.Port || a.Database != b.Database || a.User != b.User { + 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 { diff --git a/go/core/internal/database/connect_test.go b/go/core/internal/database/connect_test.go index 4e37eccd42..b556461563 100644 --- a/go/core/internal/database/connect_test.go +++ b/go/core/internal/database/connect_test.go @@ -112,10 +112,13 @@ func TestPoolConfigRefreshesFileCredentials(t *testing.T) { connConfig := config.ConnConfig.Copy() initialTLS := connConfig.TLSConfig - writeDatabaseURL(t, path, "postgres://user:password-b@database:5432/app?sslmode=require&application_name=changed") + 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) } @@ -147,7 +150,6 @@ func TestPoolConfigRejectsRotatedIdentity(t *testing.T) { {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: "user", url: "host=primary,secondary port=5432,5433 user=changed password=password-b dbname=app sslmode=disable"}, {name: "fallback", url: "host=primary,changed port=5432,5433 user=runtime password=password-b dbname=app sslmode=disable"}, } From 7f8abf71cc9e925aa36fbe4f93f7525b36434ec3 Mon Sep 17 00:00:00 2001 From: Jeremy Alvis Date: Tue, 22 Sep 2026 07:14:54 -0700 Subject: [PATCH 07/12] Require a name for the Substrate DDL Secret and plumb its pool lifetime Signed-off-by: Jeremy Alvis --- helm/kagent/templates/postgresql-secret.yaml | 7 +++++ helm/kagent/tests/postgresql_test.yaml | 31 ++++++++++++++++++++ helm/kagent/values.yaml | 7 +++++ 3 files changed, 45 insertions(+) diff --git a/helm/kagent/templates/postgresql-secret.yaml b/helm/kagent/templates/postgresql-secret.yaml index a8f7ee7e1f..442fea0f08 100644 --- a/helm/kagent/templates/postgresql-secret.yaml +++ b/helm/kagent/templates/postgresql-secret.yaml @@ -14,6 +14,13 @@ data: {{- $databaseConnectionStringSecretRef := .Values.database.postgres.secretRef | default dict -}} {{- $substratePostgres := get .Values.substrate "postgres" | default dict -}} {{- $connectionStringSecretRef := get $substratePostgres "connectionStringSecretRef" | default dict -}} +{{- $ddlConnectionStringSecretRef := get $substratePostgres "ddlConnectionStringSecretRef" | default dict -}} +{{- /* Kagent generates only the runtime connection Secret. An unnamed DDL ref + resolves to that same Secret, where the DDL key does not exist, and the + Substrate pod then fails to mount it. */ -}} +{{- if and .Values.substrate.enabled (get $ddlConnectionStringSecretRef "enabled") (not (get $ddlConnectionStringSecretRef "name")) -}} +{{- fail "substrate.postgres.ddlConnectionStringSecretRef.name is required when ddlConnectionStringSecretRef.enabled is set" -}} +{{- end -}} {{- if and .Values.substrate.enabled (get $connectionStringSecretRef "enabled") (not (get $connectionStringSecretRef "name")) }} {{- $connectionString := "" -}} {{- if get $databaseConnectionStringSecretRef "name" -}} diff --git a/helm/kagent/tests/postgresql_test.yaml b/helm/kagent/tests/postgresql_test.yaml index c5659b4018..3564833bf4 100644 --- a/helm/kagent/tests/postgresql_test.yaml +++ b/helm/kagent/tests/postgresql_test.yaml @@ -601,3 +601,34 @@ tests: - equal: path: spec.template.spec.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms[0].matchExpressions[0].key value: topology.kubernetes.io/zone + + # =========================================================================== + # Substrate DDL connection Secret + # =========================================================================== + + - it: should fail when the Substrate DDL secret ref is enabled without a name + template: postgresql-secret.yaml + set: + substrate: + enabled: true + postgres: + ddlConnectionStringSecretRef: + enabled: true + asserts: + - failedTemplate: + errorMessage: "substrate.postgres.ddlConnectionStringSecretRef.name is required when ddlConnectionStringSecretRef.enabled is set" + + - it: should render when the Substrate DDL secret ref names an existing Secret + template: postgresql-secret.yaml + set: + substrate: + enabled: true + postgres: + connectionStringSecretRef: + enabled: true + name: shared-dml-secret + ddlConnectionStringSecretRef: + enabled: true + name: shared-ddl-secret + asserts: + - notFailedTemplate: {} diff --git a/helm/kagent/values.yaml b/helm/kagent/values.yaml index 07855a03bb..98e629cf09 100644 --- a/helm/kagent/values.yaml +++ b/helm/kagent/values.yaml @@ -785,6 +785,13 @@ substrate: enabled: false name: "" key: ddlConnectionString + 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 never retires a connection and so never picks up a rotation. + 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: From 7fb9bea0b0d8bd54c1b551fe1a8f32945b001317 Mon Sep 17 00:00:00 2001 From: Jeremy Alvis Date: Tue, 22 Sep 2026 08:36:42 -0700 Subject: [PATCH 08/12] Preserve access across username rotation Signed-off-by: Jeremy Alvis --- go/core/cli/internal/db/migrate/migrate.go | 22 +++++-- .../cli/internal/db/migrate/migrate_test.go | 10 +++ go/core/internal/database/connect.go | 16 ++++- go/core/internal/database/connect_test.go | 63 ++++++++++++++++++- go/core/pkg/app/app.go | 7 ++- go/core/pkg/app/app_test.go | 4 ++ go/core/pkg/env/kagent.go | 7 +++ go/core/pkg/migrations/runner.go | 55 +++++++++++++--- go/core/pkg/migrations/runner_test.go | 42 +++++++++++++ helm/README.md | 16 ++++- .../templates/controller-configmap.yaml | 1 + .../tests/controller-deployment_test.yaml | 11 ++++ helm/kagent/values.yaml | 7 +++ 13 files changed, 238 insertions(+), 23 deletions(-) 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/internal/database/connect.go b/go/core/internal/database/connect.go index ab57a0c3b5..8a1b98681e 100644 --- a/go/core/internal/database/connect.go +++ b/go/core/internal/database/connect.go @@ -24,6 +24,7 @@ import ( // from ParseConfig unchanged (pgx library defaults). type PostgresConfig struct { URL string + Role string VectorEnabled bool MaxConns *int32 MinConns *int32 @@ -145,6 +146,9 @@ func poolConfig(cfg *PostgresConfig) (*pgxpool.Config, error) { 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 @@ -158,9 +162,17 @@ func poolConfig(cfg *PostgresConfig) (*pgxpool.Config, error) { } } - if cfg.VectorEnabled { + if cfg.Role != "" || cfg.VectorEnabled { config.AfterConnect = func(ctx context.Context, conn *pgx.Conn) error { - return pgvectorpgx.RegisterTypes(ctx, conn) + 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.VectorEnabled { + return pgvectorpgx.RegisterTypes(ctx, conn) + } + return nil } } return config, nil diff --git a/go/core/internal/database/connect_test.go b/go/core/internal/database/connect_test.go index b556461563..8f53e967a6 100644 --- a/go/core/internal/database/connect_test.go +++ b/go/core/internal/database/connect_test.go @@ -2,6 +2,7 @@ package database import ( "context" + "net/url" "os" "path/filepath" "testing" @@ -105,9 +106,10 @@ func TestPoolConfigRefreshesFileCredentials(t *testing.T) { 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}) + 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() @@ -123,6 +125,65 @@ func TestPoolConfigRefreshesFileCredentials(t *testing.T) { assert.NotSame(t, initialTLS, connConfig.TLSConfig) } +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", diff --git a/go/core/pkg/app/app.go b/go/core/pkg/app/app.go index 5be1081eab..91ca34f0a3 100644 --- a/go/core/pkg/app/app.go +++ b/go/core/pkg/app/app.go @@ -176,14 +176,15 @@ func Run(ctx context.Context, opts Options) error { 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...) 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, postgresConfigFromEnv(dbSource, vectorEnabled)) @@ -371,7 +372,7 @@ func envBool(name string) bool { } func postgresConfigFromEnv(source string, vectorEnabled bool) *database.PostgresConfig { - config := &database.PostgresConfig{URL: source, VectorEnabled: vectorEnabled} + config := &database.PostgresConfig{URL: source, Role: kagentenv.DatabaseRole.Get(), VectorEnabled: vectorEnabled} if value := kagentenv.DatabaseMaxConns.Get(); value > 0 { maxConns := int32(value) config.MaxConns = &maxConns diff --git a/go/core/pkg/app/app_test.go b/go/core/pkg/app/app_test.go index e7aab2a0d9..a9fdb6e715 100644 --- a/go/core/pkg/app/app_test.go +++ b/go/core/pkg/app/app_test.go @@ -126,11 +126,15 @@ func TestPostgresConfigFromEnv(t *testing.T) { 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") 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.MaxConns == nil || *config.MaxConns != 8 { t.Fatalf("MaxConns = %v, want 8", config.MaxConns) } diff --git a/go/core/pkg/env/kagent.go b/go/core/pkg/env/kagent.go index 45534881b0..63bb872b3a 100644 --- a/go/core/pkg/env/kagent.go +++ b/go/core/pkg/env/kagent.go @@ -120,6 +120,13 @@ var ( ComponentDatabase, ) + DatabaseRole = RegisterStringVar( + "POSTGRES_DATABASE_ROLE", + "", + "Stable PostgreSQL role assumed after authentication. Required for rotation to a different login user.", + ComponentDatabase, + ) + DatabaseMaxConns = RegisterIntVar( "DB_MAX_CONNS", 0, diff --git a/go/core/pkg/migrations/runner.go b/go/core/pkg/migrations/runner.go index 50b85995cf..593d4544bb 100644 --- a/go/core/pkg/migrations/runner.go +++ b/go/core/pkg/migrations/runner.go @@ -14,7 +14,8 @@ import ( "strconv" "strings" - _ "github.com/jackc/pgx/v5/stdlib" + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/stdlib" "github.com/pressly/goose/v3" "github.com/pressly/goose/v3/lock" ) @@ -62,13 +63,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 +95,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 +108,23 @@ 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 } - db, err := sql.Open("pgx", url) + db, err := openDB(url, role) if err != nil { return fmt.Errorf("open database: %w", err) } @@ -173,6 +186,16 @@ 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 } @@ -185,7 +208,7 @@ func WithProvider(ctx context.Context, url string, src Source, fn func(*goose.Pr } } - 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) } @@ -367,7 +390,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 +403,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,6 +431,22 @@ func checkResolvedSchemaCollisions(ctx context.Context, url string, sources []So return nil } +func openDB(url, role string) (*sql.DB, error) { + if role == "" { + return sql.Open("pgx", url) + } + config, err := pgx.ParseConfig(url) + if err != nil { + return nil, errors.New("invalid PostgreSQL connection string") + } + 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 checkPgvector(url string) error { db, err := sql.Open("pgx", url) if err != nil { diff --git a/go/core/pkg/migrations/runner_test.go b/go/core/pkg/migrations/runner_test.go index 5a04c0f202..c0a78be132 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" @@ -192,6 +193,47 @@ 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 TestBuiltinMigrationsRoundTrip(t *testing.T) { dsn := startTestDB(t) sources := BuiltinSources(true) diff --git a/helm/README.md b/helm/README.md index 782bfa0dc9..f5cae8f52f 100644 --- a/helm/README.md +++ b/helm/README.md @@ -64,6 +64,7 @@ database: secretRef: name: kagent-postgres key: connectionString + role: kagent_app bundled: enabled: false substrate: @@ -79,6 +80,8 @@ substrate: enabled: true name: substrate-ddl-postgres key: connectionString + runtimeRole: substrate_runtime + ddlRole: substrate_ddl ``` The DDL role owns the Substrate schema and performs migrations and partition @@ -100,9 +103,16 @@ embedded Substrate shares the Secret, set and DDL pools. Keep old and new credentials valid long enough for Kubernetes Secret projection and connection turnover. -Rotation may change passwords and referenced TLS material. Host, port, -fallback targets, database, and username identify the pool and require a -controller restart when changed. Direct binary deployments use +Rotation may change passwords, usernames, and referenced TLS material. For a +username-changing rotation, set `database.postgres.role` to a stable `NOLOGIN` +role and grant every incoming login membership before publishing the Secret. +Set `substrate.postgres.runtimeRole` and `substrate.postgres.ddlRole` the same +way for embedded Substrate. Neither Kagent nor either Helm chart creates these +roles or grants membership: database provisioning must create the roles before +installation, and the credential rotator must grant each incoming login before +publishing its Secret. Without stable roles, changing a username requires a +restart. Host, port, fallback targets, and database always require a restart. +Direct binary deployments use `POSTGRES_DATABASE_URL=@file:/absolute/path`; there is no separate `_FILE` environment variable. diff --git a/helm/kagent/templates/controller-configmap.yaml b/helm/kagent/templates/controller-configmap.yaml index addbcebea8..4f0c37d7c8 100644 --- a/helm/kagent/templates/controller-configmap.yaml +++ b/helm/kagent/templates/controller-configmap.yaml @@ -51,6 +51,7 @@ 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 }} {{- with .Values.database.postgres.pool }} {{- if and (hasKey . "maxConns") (ne .maxConns nil) }} DB_MAX_CONNS: {{ .maxConns | quote }} diff --git a/helm/kagent/tests/controller-deployment_test.yaml b/helm/kagent/tests/controller-deployment_test.yaml index 428d544447..26a9597901 100644 --- a/helm/kagent/tests/controller-deployment_test.yaml +++ b/helm/kagent/tests/controller-deployment_test.yaml @@ -704,6 +704,17 @@ tests: path: data.DATABASE_VECTOR_ENABLED value: "true" + - it: should set stable PostgreSQL role + template: controller-configmap.yaml + set: + database: + postgres: + role: kagent_app + asserts: + - equal: + path: data.POSTGRES_DATABASE_ROLE + value: kagent_app + - it: should not set DB pool env vars by default template: controller-configmap.yaml asserts: diff --git a/helm/kagent/values.yaml b/helm/kagent/values.yaml index 98e629cf09..9ae6ef5e8b 100644 --- a/helm/kagent/values.yaml +++ b/helm/kagent/values.yaml @@ -115,6 +115,9 @@ database: secretRef: name: "" key: connectionString + # -- Stable NOLOGIN role assumed after authentication. Required when Secret + # rotation changes the PostgreSQL login username. + role: "" # -- 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. @@ -785,6 +788,10 @@ substrate: enabled: false name: "" key: ddlConnectionString + # -- Stable NOLOGIN roles assumed after authentication. Configure these + # when rotated Substrate credentials change login usernames. + runtimeRole: "" + ddlRole: "" pool: # -- Maximum physical connection lifetime for Substrate's pools. # Bounds how long a rotated credential stays in use. Set it longer than the From dd8e5e8df2da6b0761830cca849011b05091c377 Mon Sep 17 00:00:00 2001 From: Jeremy Alvis Date: Tue, 22 Sep 2026 22:01:08 -0700 Subject: [PATCH 09/12] Share bundled PostgreSQL with Substrate using separate identities Signed-off-by: Jeremy Alvis --- go/core/cli/internal/commands/db/db.go | 2 +- go/core/cmd/controller/main.go | 48 ++ go/core/internal/database/bootstrap.go | 98 +++ go/core/internal/database/bootstrap_test.go | 284 +++++++ go/core/internal/database/client_postgres.go | 13 +- go/core/internal/database/client_test.go | 5 +- go/core/internal/database/connect.go | 43 +- go/core/internal/database/connect_test.go | 25 + go/core/internal/database/memory.go | 8 +- go/core/internal/database/sql_test.go | 28 +- go/core/internal/dbtest/dbtest.go | 15 +- go/core/pkg/app/app.go | 12 +- go/core/pkg/app/app_test.go | 4 + go/core/pkg/env/kagent.go | 14 + go/core/pkg/migrations/identity/bootstrap.sql | 75 ++ go/core/pkg/migrations/migrations.go | 2 +- go/core/pkg/migrations/runner.go | 96 ++- go/core/pkg/migrations/runner_test.go | 83 +- .../pkg/migrations/vector/000001_initial.sql | 15 +- helm/README.md | 139 ++-- helm/kagent/templates/NOTES.txt | 17 +- helm/kagent/templates/_helpers.tpl | 12 +- .../templates/controller-configmap.yaml | 2 + .../templates/controller-deployment.yaml | 75 +- helm/kagent/templates/postgresql-secret.yaml | 81 +- helm/kagent/templates/postgresql.yaml | 25 +- .../tests/controller-deployment_test.yaml | 344 ++++---- helm/kagent/tests/postgresql_test.yaml | 742 ++++++------------ helm/kagent/values.yaml | 78 +- 29 files changed, 1498 insertions(+), 887 deletions(-) create mode 100644 go/core/internal/database/bootstrap.go create mode 100644 go/core/internal/database/bootstrap_test.go create mode 100644 go/core/pkg/migrations/identity/bootstrap.sql 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/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..837b8a4dba --- /dev/null +++ b/go/core/internal/database/bootstrap_test.go @@ -0,0 +1,284 @@ +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, + VectorSchema: "extensions", + })) + 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", VectorSchema: "extensions", 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..e2d105d601 100644 --- a/go/core/internal/database/client_postgres.go +++ b/go/core/internal/database/client_postgres.go @@ -12,13 +12,18 @@ import ( // 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 public. +func NewClient(db *pgxpool.Pool, vectorSchema ...string) *Client { + schema := "public" + 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..65e2ff5450 100644 --- a/go/core/internal/database/client_test.go +++ b/go/core/internal/database/client_test.go @@ -104,9 +104,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, diff --git a/go/core/internal/database/connect.go b/go/core/internal/database/connect.go index 8a1b98681e..5fe7489c9b 100644 --- a/go/core/internal/database/connect.go +++ b/go/core/internal/database/connect.go @@ -25,6 +25,8 @@ import ( type PostgresConfig struct { URL string Role string + Schema string + VectorSchema string VectorEnabled bool MaxConns *int32 MinConns *int32 @@ -127,6 +129,21 @@ func poolConfig(cfg *PostgresConfig) (*pgxpool.Config, error) { if err := applyPoolConfig(config, cfg); err != nil { return nil, err } + vectorSchema := cfg.VectorSchema + if vectorSchema == "" { + vectorSchema = "public" + } + if cfg.VectorEnabled && cfg.Schema == "" && vectorSchema != "public" { + return nil, errors.New("database schema is required when pgvector uses a non-public schema") + } + 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) { @@ -162,15 +179,37 @@ func poolConfig(cfg *PostgresConfig) (*pgxpool.Config, error) { } } - if cfg.Role != "" || cfg.VectorEnabled { + 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 { - return pgvectorpgx.RegisterTypes(ctx, conn) + 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 } diff --git a/go/core/internal/database/connect_test.go b/go/core/internal/database/connect_test.go index 8f53e967a6..6af40dfef3 100644 --- a/go/core/internal/database/connect_test.go +++ b/go/core/internal/database/connect_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -125,6 +126,30 @@ func TestPoolConfigRefreshesFileCredentials(t *testing.T) { 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") 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/dbtest/dbtest.go b/go/core/internal/dbtest/dbtest.go index 0a3e7d7236..b280224b96 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,10 +61,20 @@ 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 { + 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.BuiltinSources(vectorEnabled)) } diff --git a/go/core/pkg/app/app.go b/go/core/pkg/app/app.go index 575a399222..f3cc15f083 100644 --- a/go/core/pkg/app/app.go +++ b/go/core/pkg/app/app.go @@ -181,7 +181,7 @@ func Run(ctx context.Context, opts Options) error { 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.VerifyMigratedAsRole(ctx, dbURL, dbRole, sources); err != nil { return fmt.Errorf("verify database migrations: %w", err) @@ -194,7 +194,7 @@ func Run(ctx context.Context, opts Options) error { return err } defer db.Close() - store := database.NewClient(db) + store := database.NewClient(db, kagentenv.DatabaseVectorSchema.Get()) kubeConfig, err := clientcmd.NewNonInteractiveDeferredLoadingClientConfig( clientcmd.NewDefaultClientConfigLoadingRules(), &clientcmd.ConfigOverrides{}, @@ -389,7 +389,13 @@ func envBool(name string) bool { } func postgresConfigFromEnv(source string, vectorEnabled bool) *database.PostgresConfig { - config := &database.PostgresConfig{URL: source, Role: kagentenv.DatabaseRole.Get(), VectorEnabled: vectorEnabled} + 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 diff --git a/go/core/pkg/app/app_test.go b/go/core/pkg/app/app_test.go index 9ad3e1de6c..6b27661111 100644 --- a/go/core/pkg/app/app_test.go +++ b/go/core/pkg/app/app_test.go @@ -159,6 +159,7 @@ func TestPostgresConfigFromEnv(t *testing.T) { 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 { @@ -167,6 +168,9 @@ func TestPostgresConfigFromEnv(t *testing.T) { 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) } diff --git a/go/core/pkg/env/kagent.go b/go/core/pkg/env/kagent.go index 32336b19e4..10d2901dad 100644 --- a/go/core/pkg/env/kagent.go +++ b/go/core/pkg/env/kagent.go @@ -145,6 +145,20 @@ var ( ComponentDatabase, ) + DatabaseSchema = RegisterStringVar( + "POSTGRES_DATABASE_SCHEMA", + "public", + "PostgreSQL schema for Kagent tables.", + ComponentDatabase, + ) + + DatabaseVectorSchema = RegisterStringVar( + "POSTGRES_VECTOR_SCHEMA", + "public", + "Schema where the shared pgvector extension is installed.", + ComponentDatabase, + ) + DatabaseMaxConns = RegisterIntVar( "DB_MAX_CONNS", 0, diff --git a/go/core/pkg/migrations/identity/bootstrap.sql b/go/core/pkg/migrations/identity/bootstrap.sql new file mode 100644 index 0000000000..4600865c4a --- /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 public). 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), ''), 'public'); + 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 593d4544bb..162cd06650 100644 --- a/go/core/pkg/migrations/runner.go +++ b/go/core/pkg/migrations/runner.go @@ -35,6 +35,7 @@ var ( type Source struct { Name string Schema string + VectorSchema string TrackingTable string FS fs.FS Dir string @@ -52,15 +53,35 @@ func BuiltinSources(vectorEnabled bool) []Source { if vectorEnabled { sources = append(sources, Source{ Name: "vector", + VectorSchema: "public", TrackingTable: vectorTrackingTable, FS: FS, Dir: "vector", - PreCheck: checkPgvector, + PreCheck: pgvectorPreCheck("public"), }) } return sources } +// BuiltinSourcesInSchema returns the built-in sources with their table and +// pgvector schemas selected independently. +func BuiltinSourcesInSchema(vectorEnabled bool, schema, vectorSchema string) []Source { + if vectorSchema == "" { + vectorSchema = "public" + } + sources := BuiltinSources(vectorEnabled) + for i := range sources { + sources[i].Schema = schema + if vectorEnabled { + sources[i].VectorSchema = vectorSchema + } + } + if vectorEnabled { + sources[1].PreCheck = pgvectorPreCheck(vectorSchema) + } + return sources +} + // RunUp applies all pending migrations in source order. func RunUp(ctx context.Context, url string, sources []Source) error { return RunUpAsRole(ctx, url, "", sources) @@ -123,6 +144,13 @@ func VerifyMigratedAsRole(ctx context.Context, url, role string, sources []Sourc 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 := openDB(url, role) if err != nil { @@ -200,9 +228,9 @@ func withProvider(ctx context.Context, url, role string, src Source, fn func(*go 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) } @@ -217,8 +245,14 @@ func withProvider(ctx context.Context, url, role string, src Source, fn func(*go }() 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) + } } } @@ -227,6 +261,9 @@ func withProvider(ctx context.Context, url, role string, src Source, fn func(*go 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") } @@ -306,6 +343,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 == "" && src.VectorSchema != "public" { + return fmt.Errorf("source %s needs a table schema when pgvector uses a non-public schema", src.Name) + } + } if err := validateIdentifier("tracking table", src.TrackingTable); err != nil { return fmt.Errorf("source %s: %w", src.Name, err) } @@ -447,23 +492,29 @@ func openDB(url, role string) (*sql.DB, error) { })), nil } -func checkPgvector(url string) error { - db, err := sql.Open("pgx", url) - if err != nil { - return fmt.Errorf("open database: %w", err) - } - 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) - } - if !available { - return errors.New("pgvector is unavailable. Install it or disable database vectors") +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) @@ -472,7 +523,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 c0a78be132..28c3bf83e4 100644 --- a/go/core/pkg/migrations/runner_test.go +++ b/go/core/pkg/migrations/runner_test.go @@ -71,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 } @@ -234,6 +235,53 @@ func TestRunUpAsStableRole(t *testing.T) { } } +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) @@ -508,15 +556,42 @@ func TestBuiltinTrackingTables(t *testing.T) { } } +func TestBuiltinSourcesInSchema(t *testing.T) { + sources := BuiltinSourcesInSchema(true, "kagent", "extensions") + for _, source := range sources { + if source.Schema != "kagent" { + t.Fatalf("source %q schema = %q", source.Name, source.Schema) + } + if source.VectorSchema != "extensions" { + t.Fatalf("source %q vector schema = %q", source.Name, source.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..9181bdea5d 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), ''), 'public'); +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/helm/README.md b/helm/README.md index f5cae8f52f..8713aaa8fd 100644 --- a/helm/README.md +++ b/helm/README.md @@ -23,40 +23,51 @@ helm install kagent ./helm/kagent/ --namespace kagent --set providers.default=az ### Substrate PostgreSQL -Kagent supports three PostgreSQL layouts with embedded Substrate. - -1. Share Kagent's bundled PostgreSQL. Kagent and Substrate use the same - database with separate schemas; the parent chart creates Substrate's - release-scoped connection Secret. +The default install uses one PostgreSQL instance and one `kagent` database. +Kagent uses the `public` 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 `public`). All Kagent installs +using the same database must select that schema. For an external database, +install pgvector there before running migrations and 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 ``` -2. Share one external Secret. Helm cannot dynamically copy a parent Secret - reference into a dependency, so repeat the same name and key explicitly. +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. -```yaml -database: - postgres: - secretRef: - name: shared-postgres - key: connectionString - bundled: - enabled: false -substrate: - enabled: true - postgres: - enabled: false - connectionStringSecretRef: - enabled: true - name: shared-postgres - key: connectionString -``` +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. -3. Use separate Kagent, Substrate runtime/DML, and Substrate DDL/maintenance - Secrets. +For an external database, create the users, roles, schemas, and grants yourself, then provide three application connection Secrets: ```yaml database: @@ -64,64 +75,56 @@ database: secretRef: name: kagent-postgres key: connectionString - role: kagent_app bundled: enabled: false substrate: enabled: true postgres: enabled: false - schema: substrate - connectionStringSecretRef: - enabled: true - name: substrate-runtime-postgres - key: connectionString - ddlConnectionStringSecretRef: - enabled: true - name: substrate-ddl-postgres - key: connectionString - runtimeRole: substrate_runtime - ddlRole: substrate_ddl + readWriteConnectionStringSecretRef: + name: substrate-postgres-readwrite + key: readWriteConnectionString + ownerConnectionStringSecretRef: + name: substrate-postgres-owner + key: ownerConnectionString + bootstrap: false ``` -The DDL role owns the Substrate schema and performs migrations and partition -maintenance. Substrate grants its runtime role access to migrated tables and -sequences. Omitting the DDL connection preserves single-connection operation. +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. -An inline `database.postgres.url` remains supported, but is fixed for the life -of the controller process. When embedded Substrate is enabled, the parent chart -can copy that inline value into its release-scoped Substrate Secret. +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 -`database.postgres.secretRef` is mounted through a Secret volume. Kagent -rereads the connection string before opening each new physical connection; -existing sessions remain valid until pgx retires them. Set -`database.postgres.pool.maxConnLifetime` to bound Kagent's turnover time. When -embedded Substrate shares the Secret, set -`substrate.postgres.pool.maxConnLifetime` as well to bound its runtime, watch, -and DDL pools. Keep old and new credentials valid long enough for Kubernetes -Secret projection and connection turnover. - -Rotation may change passwords, usernames, and referenced TLS material. For a -username-changing rotation, set `database.postgres.role` to a stable `NOLOGIN` -role and grant every incoming login membership before publishing the Secret. -Set `substrate.postgres.runtimeRole` and `substrate.postgres.ddlRole` the same -way for embedded Substrate. Neither Kagent nor either Helm chart creates these -roles or grants membership: database provisioning must create the roles before -installation, and the credential rotator must grant each incoming login before -publishing its Secret. Without stable roles, changing a username requires a -restart. Host, port, fallback targets, and database always require a restart. -Direct binary deployments use -`POSTGRES_DATABASE_URL=@file:/absolute/path`; there is no separate `_FILE` -environment variable. - -Kagent 1.x removes `database.postgres.urlFile`. Replace: +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: - urlFile: /user-managed/path + url: postgresql://user:password@database.example/kagent ``` with: diff --git a/helm/kagent/templates/NOTES.txt b/helm/kagent/templates/NOTES.txt index 304d8a84c4..5f9afa86f1 100644 --- a/helm/kagent/templates/NOTES.txt +++ b/helm/kagent/templates/NOTES.txt @@ -62,28 +62,15 @@ DOCUMENTATION: {{- end }} {{ if .Values.database.postgres.bundled.enabled -}} ################################################################################ -{{- if and (eq .Values.database.postgres.url "") (not .Values.database.postgres.secretRef.name) }} # 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.secretRef.name= -{{- 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. - - To connect the controller to the bundled instance instead, unset the external connection: - database.postgres.url="" - database.postgres.secretRef.name="" - 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 4f0c37d7c8..5b300c8547 100644 --- a/helm/kagent/templates/controller-configmap.yaml +++ b/helm/kagent/templates/controller-configmap.yaml @@ -52,6 +52,8 @@ data: 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 6686ed83c3..c262fe6a15 100644 --- a/helm/kagent/templates/controller-deployment.yaml +++ b/helm/kagent/templates/controller-deployment.yaml @@ -1,12 +1,29 @@ {{- $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 and .Values.database.postgres.url (get $databaseConnectionStringSecretRef "name") -}} -{{- fail "database.postgres.url and database.postgres.secretRef.name are mutually exclusive" -}} +{{- 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 @@ -45,15 +62,26 @@ spec: {{- toYaml . | nindent 8 }} {{- end }} serviceAccountName: {{ include "kagent.fullname" . }}-controller - {{- if or (get $databaseConnectionStringSecretRef "name") (gt (len .Values.controller.volumes) 0) (and .Values.controller.substrate .Values.controller.substrate.enabled) }} volumes: - {{- if get $databaseConnectionStringSecretRef "name" }} - name: {{ $databaseSecretVolumeName }} - secret: - secretName: {{ get $databaseConnectionStringSecretRef "name" }} - items: - - key: {{ get $databaseConnectionStringSecretRef "key" | default "connectionString" }} - path: {{ $databaseSecretFileName }} + 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 @@ -76,7 +104,6 @@ spec: {{- with .Values.controller.volumes }} {{- toYaml . | nindent 6 }} {{- end }} - {{- end }} {{- with .Values.controller.nodeSelector }} nodeSelector: {{- toYaml . | nindent 8 }} @@ -116,22 +143,15 @@ spec: - name: AUTH_USER_ID_CLAIM value: {{ .Values.controller.auth.userIdClaim | quote }} {{- end }} - {{- if get $databaseConnectionStringSecretRef "name" }} - name: POSTGRES_DATABASE_URL value: {{ printf "@file:%s/%s" $databaseSecretMountPath $databaseSecretFileName | 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.secretRef.name, or enable database.postgres.bundled." }} + - 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 @@ -212,12 +232,14 @@ spec: port: http periodSeconds: 30 {{- end }} - {{- if or (get $databaseConnectionStringSecretRef "name") (gt (len .Values.controller.volumeMounts) 0) (and .Values.controller.substrate .Values.controller.substrate.enabled) }} volumeMounts: - {{- if get $databaseConnectionStringSecretRef "name" }} - 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 @@ -230,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 442fea0f08..524acdd06a 100644 --- a/helm/kagent/templates/postgresql-secret.yaml +++ b/helm/kagent/templates/postgresql-secret.yaml @@ -1,47 +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 }} -{{- $databaseConnectionStringSecretRef := .Values.database.postgres.secretRef | default dict -}} -{{- $substratePostgres := get .Values.substrate "postgres" | default dict -}} -{{- $connectionStringSecretRef := get $substratePostgres "connectionStringSecretRef" | default dict -}} -{{- $ddlConnectionStringSecretRef := get $substratePostgres "ddlConnectionStringSecretRef" | default dict -}} -{{- /* Kagent generates only the runtime connection Secret. An unnamed DDL ref - resolves to that same Secret, where the DDL key does not exist, and the - Substrate pod then fails to mount it. */ -}} -{{- if and .Values.substrate.enabled (get $ddlConnectionStringSecretRef "enabled") (not (get $ddlConnectionStringSecretRef "name")) -}} -{{- fail "substrate.postgres.ddlConnectionStringSecretRef.name is required when ddlConnectionStringSecretRef.enabled is set" -}} -{{- end -}} -{{- if and .Values.substrate.enabled (get $connectionStringSecretRef "enabled") (not (get $connectionStringSecretRef "name")) }} -{{- $connectionString := "" -}} -{{- if get $databaseConnectionStringSecretRef "name" -}} -{{- fail "database.postgres.secretRef cannot be inherited by Substrate; set substrate.postgres.connectionStringSecretRef to the same Secret" -}} -{{- else if .Values.database.postgres.url -}} -{{- $connectionString = .Values.database.postgres.url -}} -{{- else if .Values.database.postgres.bundled.enabled -}} -{{- $connectionString = printf "postgres://kagent:kagent@%s.%s.svc:5432/kagent?sslmode=disable" (include "kagent.postgresqlServiceName" .) (include "kagent.namespace" .) -}} -{{- else -}} -{{- fail "No database connection configured. Set database.postgres.url, substrate.postgres.connectionStringSecretRef.name, or enable database.postgres.bundled." -}} +{{- 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 $connectionStringSecretRef "name" | default (include "substrate.fullname" (list "postgres-connection" .)) }} + 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 $connectionStringSecretRef "key" | default "connectionString" }}: {{ $connectionString | quote }} + {{ 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 26a9597901..2ca7b74869 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,105 +490,114 @@ 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: - - contains: - path: spec.template.spec.containers[0].env - content: - name: POSTGRES_PASSWORD - valueFrom: - secretKeyRef: - name: RELEASE-NAME-postgresql - key: POSTGRES_PASSWORD - - 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" + - failedTemplate: + errorMessage: database.postgres.bundled.bootstrap must be true or false - - it: should set POSTGRES_DATABASE_URL with external url when url is set + - it: should reject the old bootstrap location template: controller-deployment.yaml set: - database: - postgres: - url: "postgres://user:pass@external-host:5432/db" + 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_DATABASE_URL - value: "postgres://user:pass@external-host:5432/db" + name: KAGENT_DATABASE_BOOTSTRAP + value: "false" + - notExists: + path: spec.template.spec.volumes[?(@.name == "postgres-admin")] - - it: should keep Kagent's bundled database configuration when Substrate is enabled + - it: should require an application Secret when bundled bootstrap is disabled template: controller-deployment.yaml set: - substrate: - enabled: true + 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_PASSWORD - valueFrom: - secretKeyRef: - name: RELEASE-NAME-postgresql - key: POSTGRES_PASSWORD + name: POSTGRES_DATABASE_URL + 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: POSTGRES_DATABASE_URL - value: "postgres://kagent:$(POSTGRES_PASSWORD)@RELEASE-NAME-postgresql.NAMESPACE.svc:5432/kagent?sslmode=disable" + 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 read an existing shared database secret with Substrate + - it: should use an external database Secret template: controller-deployment.yaml set: database: postgres: secretRef: - name: shared-db - key: url - substrate: - enabled: true - postgres: - connectionStringSecretRef: - name: shared-db - key: url + 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.volumes - content: - name: postgres-connection - secret: - secretName: shared-db - items: - - key: url - path: connection-string - - contains: - path: spec.template.spec.containers[0].volumeMounts + path: spec.template.spec.containers[0].env content: - name: postgres-connection - mountPath: /var/run/secrets/kagent/postgres - readOnly: true - - notExists: - path: spec.template.spec.containers[0].volumeMounts[0].subPath + 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 @@ -577,18 +605,15 @@ tests: substrate: enabled: true postgres: - connectionStringSecretRef: - enabled: false + bootstrap: false + readWriteConnectionStringSecretRef: name: substrate-db asserts: - contains: path: spec.template.spec.containers[0].env content: - name: POSTGRES_PASSWORD - valueFrom: - secretKeyRef: - name: RELEASE-NAME-postgresql - key: POSTGRES_PASSWORD + name: POSTGRES_DATABASE_URL + value: "@file:/var/run/secrets/kagent/postgres/connection-string" - notContains: path: spec.template.spec.containers[0].env content: @@ -603,6 +628,8 @@ tests: set: database: postgres: + bundled: + enabled: false secretRef: name: external-postgres key: url @@ -624,11 +651,13 @@ tests: path: spec.template.spec.volumes content: name: postgres-connection - secret: - secretName: external-postgres - items: - - key: url - path: connection-string + projected: + sources: + - secret: + name: external-postgres + items: + - key: url + path: connection-string - contains: path: spec.template.spec.containers[0].volumeMounts content: @@ -641,8 +670,15 @@ tests: 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 runtime and DDL Secrets + - it: should keep Kagent separate from Substrate read/write and owner Secrets template: controller-deployment.yaml set: database: @@ -655,36 +691,37 @@ tests: substrate: enabled: true postgres: - connectionStringSecretRef: - enabled: true - name: substrate-runtime-db - key: runtime-url - ddlConnectionStringSecretRef: - enabled: true - name: substrate-ddl-db - key: ddl-url + 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 - secret: - secretName: kagent-db - items: - - key: kagent-url - path: connection-string + 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-runtime-db + secretName: substrate-readwrite-db - notContains: path: spec.template.spec.volumes content: name: postgres-connection secret: - secretName: substrate-ddl-db + secretName: substrate-owner-db - it: should set DATABASE_VECTOR_ENABLED to false by default template: controller-configmap.yaml @@ -704,16 +741,48 @@ tests: path: data.DATABASE_VECTOR_ENABLED value: "true" - - it: should set stable PostgreSQL role + - 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: - role: kagent_app + database.postgres.bundled.bootstrap: false + database.postgres.secretRef.name: tenant-postgres + database.postgres.role: tenant_owner asserts: - equal: path: data.POSTGRES_DATABASE_ROLE - value: kagent_app + value: tenant_owner + + - it: should use the public PostgreSQL schema by default + template: controller-configmap.yaml + asserts: + - equal: + path: data.POSTGRES_DATABASE_SCHEMA + value: public + + - 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 configure the pgvector extension schema + template: controller-configmap.yaml + set: + database.postgres.vectorSchema: extensions + asserts: + - equal: + path: data.POSTGRES_VECTOR_SCHEMA + value: extensions - it: should not set DB pool env vars by default template: controller-configmap.yaml @@ -757,33 +826,15 @@ tests: - notExists: path: data.POSTGRES_DATABASE_URL - - it: should not set POSTGRES_PASSWORD when url is set and bundled is enabled - 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 + - it: should reject the removed url value 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 + - failedTemplate: + errorMessage: "database.postgres.url has been removed; use database.postgres.secretRef.{name,key}" - it: should reject the removed urlFile value template: controller-deployment.yaml @@ -795,37 +846,6 @@ tests: - failedTemplate: errorMessage: "database.postgres.urlFile has been removed; use database.postgres.secretRef.{name,key}" - - it: should reject both an inline URL and Secret reference - template: controller-deployment.yaml - set: - database: - postgres: - url: postgres://user:pass@external-host:5432/db - secretRef: - name: external-postgres - asserts: - - failedTemplate: - errorMessage: "database.postgres.url and database.postgres.secretRef.name are mutually exclusive" - - - it: should set external POSTGRES_DATABASE_URL and omit POSTGRES_PASSWORD when url and bundled are both enabled - template: controller-deployment.yaml - set: - database: - postgres: - url: "postgres://user:pass@external-host:5432/db" - bundled: - enabled: true - 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 - - it: should use default httpGet startup probe template: controller-deployment.yaml asserts: diff --git a/helm/kagent/tests/postgresql_test.yaml b/helm/kagent/tests/postgresql_test.yaml index 3564833bf4..fea2e87ed7 100644 --- a/helm/kagent/tests/postgresql_test.yaml +++ b/helm/kagent/tests/postgresql_test.yaml @@ -3,632 +3,402 @@ templates: - postgresql.yaml - postgresql-secret.yaml tests: - # ============================================================================= - # bundled mode (default — no external connection, 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 an external Secret is set and bundled is enabled - template: postgresql.yaml - set: - database: - postgres: - secretRef: - name: external-postgres - 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 - 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: - - equal: - path: spec.strategy.type - value: Recreate - - - it: should render Deployment with security context - template: postgresql.yaml - documentIndex: 2 + database.postgres.bundled: + storage: 10Gi + storageClassName: database 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 - - equal: - path: spec.template.spec.securityContext.seccompProfile.type - value: RuntimeDefault + path: spec.resources.requests.storage + value: 10Gi - 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: spec.storageClassName + value: database - - it: should allow bundled postgres pod security context override - template: postgresql.yaml - documentIndex: 2 - set: - database: - postgres: - bundled: - podSecurityContext: - fsGroup: 1001 - runAsUser: 1001 - runAsGroup: 1001 - runAsNonRoot: true - seccompProfile: - type: Localhost - localhostProfile: profiles/postgres.json + - it: should create the default administrator Secret + template: postgresql-secret.yaml + documentIndex: 0 asserts: - 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 - equal: - path: spec.template.spec.securityContext.seccompProfile.type - value: Localhost + path: stringData.POSTGRES_USER + value: postgres - equal: - path: spec.template.spec.securityContext.seccompProfile.localhostProfile - value: profiles/postgres.json + path: stringData.POSTGRES_PASSWORD + value: postgres - - it: should allow bundled postgres container 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: - securityContext: - allowPrivilegeEscalation: false - readOnlyRootFilesystem: false - capabilities: - drop: - - ALL - seccompProfile: - type: Localhost - localhostProfile: profiles/postgres-container.json + database.postgres.bundled.bootstrap: false + database.postgres.secretRef.name: existing-kagent asserts: + - hasDocuments: + count: 1 - 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 - - equal: - path: spec.template.spec.containers[0].securityContext.seccompProfile.localhostProfile - value: profiles/postgres-container.json + path: metadata.name + value: postgres-admin - - it: should render Service with hardcoded ClusterIP type and port 5432 - template: postgresql.yaml - documentIndex: 3 + - it: should create the default Kagent Secret + template: postgresql-secret.yaml + documentIndex: 1 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: kagent-postgres - equal: - path: spec.selector["app.kubernetes.io/component"] - value: database + path: stringData.connectionString + value: postgresql://kagent_user:kagent@RELEASE-NAME-postgresql.NAMESPACE.svc:5432/kagent?sslmode=disable - - it: should set serviceAccountName on Deployment - template: postgresql.yaml + - it: should create the Substrate read/write Secret + template: postgresql-secret.yaml documentIndex: 2 + set: + substrate.enabled: true asserts: - - equal: - path: spec.template.spec.serviceAccountName - value: RELEASE-NAME-postgresql - - - it: should create ServiceAccount when bundled is enabled - template: postgresql.yaml - documentIndex: 0 - asserts: - - isKind: - of: ServiceAccount - equal: path: metadata.name - value: RELEASE-NAME-postgresql - - # ============================================================================= - # postgresql-secret.yaml - # ============================================================================= + value: substrate-postgres-readwrite + - equal: + path: stringData.readWriteConnectionString + value: postgresql://substrate_readwrite_user:substrate-readwrite@RELEASE-NAME-postgresql.NAMESPACE.svc:5432/kagent?sslmode=disable - - it: should create secret in bundled mode + - it: should create the Substrate owner Secret template: postgresql-secret.yaml + documentIndex: 3 + set: + substrate.enabled: true asserts: - - isKind: - of: Secret - equal: path: metadata.name - value: RELEASE-NAME-postgresql - - isNotNull: - path: data.POSTGRES_PASSWORD - - - it: should base64-encode the hardcoded demo password - template: postgresql-secret.yaml - asserts: + value: substrate-postgres-owner - equal: - path: data.POSTGRES_PASSWORD - # echo -n "kagent" | base64 - value: "a2FnZW50" + path: stringData.ownerConnectionString + value: postgresql://substrate_admin_user:substrate-admin@RELEASE-NAME-postgresql.NAMESPACE.svc:5432/kagent?sslmode=disable - - it: should not create secret when bundled is disabled + - it: should pass the bundled database address to Substrate with a custom release name template: postgresql-secret.yaml + documentIndex: 2 set: - database: - postgres: - bundled: - enabled: false + fullnameOverride: custom-kagent + substrate.enabled: true asserts: - - hasDocuments: - count: 0 + - equal: + path: stringData.readWriteConnectionString + value: postgresql://substrate_readwrite_user:substrate-readwrite@custom-kagent-postgresql.NAMESPACE.svc:5432/kagent?sslmode=disable - - it: should still create secret when url is set and bundled is enabled + - it: should reject a different Substrate database for shared bundled PostgreSQL template: postgresql-secret.yaml set: - database: - postgres: - url: "postgres://user:pass@host:5432/db" + substrate.enabled: true + substrate.postgres.database: shared-db asserts: - - hasDocuments: - count: 1 + - failedTemplate: + errorMessage: substrate.postgres.database must be kagent when sharing Kagent's bundled PostgreSQL - - it: should still create the bundled password secret when an external Secret is set + - it: should reject mismatched administrator Secret references for shared bundled PostgreSQL template: postgresql-secret.yaml set: - database: - postgres: - secretRef: - name: external-postgres + substrate.enabled: true + database.postgres.bundled.adminSecretRef.name: custom-admin asserts: - - hasDocuments: - count: 1 + - failedTemplate: + errorMessage: substrate.postgres.adminSecretRef must match database.postgres.bundled.adminSecretRef when sharing bundled PostgreSQL - - it: should create a shared Substrate connection secret for bundled postgres + - it: should accept matching custom administrator Secret references template: postgresql-secret.yaml documentIndex: 1 set: - substrate: - enabled: true + substrate.enabled: true + database.postgres.bundled.adminSecretRef.name: custom-admin + substrate.postgres.adminSecretRef.name: custom-admin asserts: - - isKind: - of: Secret - equal: path: metadata.name - value: RELEASE-NAME-postgres-connection - - equal: - path: stringData.connectionString - value: "postgres://kagent:kagent@RELEASE-NAME-postgresql.NAMESPACE.svc:5432/kagent?sslmode=disable" + value: substrate-postgres-readwrite - - it: should put an external URL in the shared Substrate connection secret + - it: should reject operator-managed Substrate Secrets during bootstrap template: postgresql-secret.yaml set: - database: - postgres: - url: "postgres://user:pass@external-host:5432/db" - bundled: - enabled: false - substrate: - enabled: true + substrate.enabled: true + substrate.postgres.ownerConnectionStringSecretRef.name: custom-owner asserts: - - equal: - path: stringData.connectionString - value: "postgres://user:pass@external-host:5432/db" + - failedTemplate: + errorMessage: substrate.postgres.bootstrap requires the chart-managed Substrate application Secret names; disable bootstrap for operator-managed Secrets - - it: should use an existing Substrate connection secret without creating one + - it: should not create Secrets for an external managed database template: postgresql-secret.yaml set: - database: - postgres: - bundled: - enabled: false - substrate: - enabled: true - postgres: - connectionStringSecretRef: - name: shared-db + database.postgres: + bundled: + enabled: false + secretRef: + name: kagent-database asserts: - hasDocuments: count: 0 - - it: should leave operator-provided runtime and DDL secrets unmanaged + - it: should keep operator Secrets unmanaged template: postgresql-secret.yaml set: - database: - postgres: - bundled: - enabled: false - substrate: - enabled: true - postgres: - connectionStringSecretRef: - name: substrate-database - key: runtimeUrl - ddlConnectionStringSecretRef: - name: substrate-database - key: ddlUrl + database.postgres.bundled.bootstrap: false + database.postgres.bundled.adminSecretRef.name: custom-admin + database.postgres.secretRef.name: custom-kagent asserts: - hasDocuments: count: 0 - - it: should require an explicit Substrate reference for an existing Kagent Secret - template: postgresql-secret.yaml - set: - database: - postgres: - secretRef: - name: shared-db - bundled: - enabled: false - substrate: - enabled: true - asserts: - - failedTemplate: - errorMessage: "database.postgres.secretRef cannot be inherited by Substrate; set substrate.postgres.connectionStringSecretRef to the same Secret" - - - 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 - # =========================================================================== - # Substrate DDL connection Secret - # =========================================================================== + - 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 fail when the Substrate DDL secret ref is enabled without a name - template: postgresql-secret.yaml - set: - substrate: - enabled: true - postgres: - ddlConnectionStringSecretRef: - enabled: true + - it: should use the database ServiceAccount + template: postgresql.yaml + documentIndex: 2 asserts: - - failedTemplate: - errorMessage: "substrate.postgres.ddlConnectionStringSecretRef.name is required when ddlConnectionStringSecretRef.enabled is set" + - equal: + path: spec.template.spec.serviceAccountName + value: RELEASE-NAME-postgresql - - it: should render when the Substrate DDL secret ref names an existing Secret - template: postgresql-secret.yaml - set: - substrate: - enabled: true - postgres: - connectionStringSecretRef: - enabled: true - name: shared-dml-secret - ddlConnectionStringSecretRef: - enabled: true - name: shared-ddl-secret + - it: should omit optional pod fields by default + template: postgresql.yaml + documentIndex: 2 asserts: - - notFailedTemplate: {} + - 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 9ae6ef5e8b..41a4be9045 100644 --- a/helm/kagent/values.yaml +++ b/helm/kagent/values.yaml @@ -104,20 +104,23 @@ nodeSelector: {} database: postgres: - # -- External PostgreSQL connection string. - # Mutually exclusive with `secretRef.name`. - url: "" - # -- Source the external PostgreSQL connection string from an existing Secret. + # -- 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. - # To share it with embedded Substrate, set the same name and key under - # `substrate.postgres.connectionStringSecretRef`. + # Embedded Substrate uses separate read/write and owner connection Secrets + # even when it shares the same database. secretRef: name: "" key: connectionString - # -- Stable NOLOGIN role assumed after authentication. Required when Secret - # rotation changes the PostgreSQL login username. - role: "" + # -- 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: public + # -- 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: public # -- 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. @@ -139,6 +142,15 @@ database: bundled: # -- 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 @@ -156,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: @@ -771,33 +782,34 @@ substrate: # Kagent and Substrate use separate schemas in the same database. enabled: false schema: substrate - # -- Optional inline Substrate runtime/DML connection string. - # Disable connectionStringSecretRef when using it. - connectionString: "" - # -- Read the Substrate runtime/DML connection string from a Secret. - # With no name, Kagent creates this Secret from its bundled or inline URL. - connectionStringSecretRef: - enabled: true - # -- Existing Secret name. Kagent creates a release-named Secret when empty. - name: "" - key: connectionString - # -- Optional DDL and maintenance connection. Defaults to the runtime connection. - ddlConnectionString: "" - # -- Read the optional DDL connection string from a Secret. - ddlConnectionStringSecretRef: - enabled: false - name: "" - key: ddlConnectionString - # -- Stable NOLOGIN roles assumed after authentication. Configure these - # when rotated Substrate credentials change login usernames. - runtimeRole: "" - ddlRole: "" + # -- 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 never retires a connection and so never picks up a rotation. + # 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. From 39599b1131ac7e97d9407d779dcb83cca09aea45 Mon Sep 17 00:00:00 2001 From: Jeremy Alvis Date: Wed, 23 Sep 2026 10:14:27 -0700 Subject: [PATCH 10/12] Update default schema to 'kagent' and default vector schema to 'extensions' Signed-off-by: Jeremy Alvis --- go/core/internal/database/bootstrap_test.go | 3 +- go/core/internal/database/client_postgres.go | 4 +- go/core/internal/database/client_test.go | 23 ++++++---- go/core/internal/database/connect.go | 7 +-- go/core/internal/database/connect_test.go | 11 +++++ go/core/internal/database/testhelpers_test.go | 2 +- go/core/internal/dbtest/dbtest.go | 2 +- go/core/pkg/env/kagent.go | 4 +- go/core/pkg/migrations/identity/bootstrap.sql | 4 +- go/core/pkg/migrations/runner.go | 44 +++++++++---------- go/core/pkg/migrations/runner_test.go | 16 +++++-- .../pkg/migrations/vector/000001_initial.sql | 2 +- go/core/test/upgrade/roundtrip_test.go | 4 +- helm/README.md | 7 +-- .../tests/controller-deployment_test.yaml | 17 ++++--- helm/kagent/values.yaml | 4 +- 16 files changed, 91 insertions(+), 63 deletions(-) diff --git a/go/core/internal/database/bootstrap_test.go b/go/core/internal/database/bootstrap_test.go index 837b8a4dba..c638daac58 100644 --- a/go/core/internal/database/bootstrap_test.go +++ b/go/core/internal/database/bootstrap_test.go @@ -146,7 +146,6 @@ func TestBootstrapSharesPgvectorAcrossSchemas(t *testing.T) { AdminPassword: adminConfig.Password, Schema: schema, VectorEnabled: true, - VectorSchema: "extensions", })) require.NoError(t, migrations.RunUpAsRole(t.Context(), appDSN.String(), OwnerRoleName, migrations.BuiltinSourcesInSchema(true, schema, "extensions"))) @@ -173,7 +172,7 @@ func TestBootstrapSharesPgvectorAcrossSchemas(t *testing.T) { assert.False(t, thirdSchemaExists) pool, err := Connect(t.Context(), &PostgresConfig{ - URL: appDSN.String(), Role: OwnerRoleName, Schema: "tenant_two", VectorSchema: "extensions", VectorEnabled: true, + URL: appDSN.String(), Role: OwnerRoleName, Schema: "tenant_two", VectorEnabled: true, }) require.NoError(t, err) defer pool.Close() diff --git a/go/core/internal/database/client_postgres.go b/go/core/internal/database/client_postgres.go index e2d105d601..f328bd6170 100644 --- a/go/core/internal/database/client_postgres.go +++ b/go/core/internal/database/client_postgres.go @@ -17,9 +17,9 @@ type Client struct { } // NewClient wraps an existing PostgreSQL pool without connecting or migrating. The caller -// owns the pool and must close it. The optional pgvector schema defaults to public. +// owns the pool and must close it. The optional pgvector schema defaults to extensions. func NewClient(db *pgxpool.Pool, vectorSchema ...string) *Client { - schema := "public" + schema := "extensions" if len(vectorSchema) > 0 && vectorSchema[0] != "" { schema = vectorSchema[0] } diff --git a/go/core/internal/database/client_test.go b/go/core/internal/database/client_test.go index 65e2ff5450..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')`) @@ -133,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" @@ -182,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" @@ -206,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" @@ -246,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)} @@ -265,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)} @@ -281,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" @@ -315,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" @@ -364,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 5fe7489c9b..84c9941704 100644 --- a/go/core/internal/database/connect.go +++ b/go/core/internal/database/connect.go @@ -22,6 +22,7 @@ import ( // 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 @@ -131,10 +132,10 @@ func poolConfig(cfg *PostgresConfig) (*pgxpool.Config, error) { } vectorSchema := cfg.VectorSchema if vectorSchema == "" { - vectorSchema = "public" + vectorSchema = "extensions" } - if cfg.VectorEnabled && cfg.Schema == "" && vectorSchema != "public" { - return nil, errors.New("database schema is required when pgvector uses a non-public schema") + if cfg.VectorEnabled && cfg.Schema == "" { + return nil, errors.New("database schema is required when pgvector is enabled") } var searchPath string if cfg.Schema != "" { diff --git a/go/core/internal/database/connect_test.go b/go/core/internal/database/connect_test.go index 6af40dfef3..0e662f5d8c 100644 --- a/go/core/internal/database/connect_test.go +++ b/go/core/internal/database/connect_test.go @@ -292,6 +292,8 @@ func TestPoolConfigPreservesHooksAndLimits(t *testing.T) { 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, @@ -307,6 +309,15 @@ func TestPoolConfigPreservesHooksAndLimits(t *testing.T) { 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/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 b280224b96..1ad8df9ea3 100644 --- a/go/core/internal/dbtest/dbtest.go +++ b/go/core/internal/dbtest/dbtest.go @@ -75,7 +75,7 @@ func Migrate(connStr string, vectorEnabled bool) error { return err } } - return migrations.RunUp(context.Background(), connStr, migrations.BuiltinSources(vectorEnabled)) + 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/env/kagent.go b/go/core/pkg/env/kagent.go index 10d2901dad..b9941acdde 100644 --- a/go/core/pkg/env/kagent.go +++ b/go/core/pkg/env/kagent.go @@ -147,14 +147,14 @@ var ( DatabaseSchema = RegisterStringVar( "POSTGRES_DATABASE_SCHEMA", - "public", + "kagent", "PostgreSQL schema for Kagent tables.", ComponentDatabase, ) DatabaseVectorSchema = RegisterStringVar( "POSTGRES_VECTOR_SCHEMA", - "public", + "extensions", "Schema where the shared pgvector extension is installed.", ComponentDatabase, ) diff --git a/go/core/pkg/migrations/identity/bootstrap.sql b/go/core/pkg/migrations/identity/bootstrap.sql index 4600865c4a..d02a79c6fb 100644 --- a/go/core/pkg/migrations/identity/bootstrap.sql +++ b/go/core/pkg/migrations/identity/bootstrap.sql @@ -2,7 +2,7 @@ -- 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 public). Optionally set +-- 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. @@ -15,7 +15,7 @@ DECLARE 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), ''), 'public'); + vector_schema text := COALESCE(NULLIF(current_setting('kagent.bootstrap_vector_schema', true), ''), 'extensions'); installed_vector_schema text; role_attrs record; schema_owner text; diff --git a/go/core/pkg/migrations/runner.go b/go/core/pkg/migrations/runner.go index 162cd06650..ea9b23294e 100644 --- a/go/core/pkg/migrations/runner.go +++ b/go/core/pkg/migrations/runner.go @@ -42,46 +42,42 @@ type Source struct { 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, "kagent", "extensions") +} + +// 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 = "kagent" + } + if vectorSchema == "" { + vectorSchema = "extensions" + } sources := []Source{{ Name: "core", + Schema: schema, TrackingTable: coreTrackingTable, FS: FS, Dir: "core", }} if vectorEnabled { + sources[0].VectorSchema = vectorSchema sources = append(sources, Source{ Name: "vector", - VectorSchema: "public", + Schema: schema, + VectorSchema: vectorSchema, TrackingTable: vectorTrackingTable, FS: FS, Dir: "vector", - PreCheck: pgvectorPreCheck("public"), + PreCheck: pgvectorPreCheck(vectorSchema), }) } return sources } -// BuiltinSourcesInSchema returns the built-in sources with their table and -// pgvector schemas selected independently. -func BuiltinSourcesInSchema(vectorEnabled bool, schema, vectorSchema string) []Source { - if vectorSchema == "" { - vectorSchema = "public" - } - sources := BuiltinSources(vectorEnabled) - for i := range sources { - sources[i].Schema = schema - if vectorEnabled { - sources[i].VectorSchema = vectorSchema - } - } - if vectorEnabled { - sources[1].PreCheck = pgvectorPreCheck(vectorSchema) - } - return sources -} - // RunUp applies all pending migrations in source order. func RunUp(ctx context.Context, url string, sources []Source) error { return RunUpAsRole(ctx, url, "", sources) @@ -347,8 +343,8 @@ func validateSources(sources []Source) error { if err := validateIdentifier("pgvector schema", src.VectorSchema); err != nil { return fmt.Errorf("source %s: %w", src.Name, err) } - if src.Schema == "" && src.VectorSchema != "public" { - return fmt.Errorf("source %s needs a table schema when pgvector uses a non-public schema", src.Name) + 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 { diff --git a/go/core/pkg/migrations/runner_test.go b/go/core/pkg/migrations/runner_test.go index 28c3bf83e4..38a0fe7fe6 100644 --- a/go/core/pkg/migrations/runner_test.go +++ b/go/core/pkg/migrations/runner_test.go @@ -284,7 +284,7 @@ func TestPgvectorSchemaMismatchFailsBeforeMigrations(t *testing.T) { 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) @@ -529,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 { @@ -554,18 +555,25 @@ 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, "kagent", "extensions") + sources := BuiltinSourcesInSchema(true, "tenant_schema", "shared_extensions") for _, source := range sources { - if source.Schema != "kagent" { + if source.Schema != "tenant_schema" { t.Fatalf("source %q schema = %q", source.Name, source.Schema) } - if source.VectorSchema != "extensions" { + 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) { diff --git a/go/core/pkg/migrations/vector/000001_initial.sql b/go/core/pkg/migrations/vector/000001_initial.sql index 9181bdea5d..ad0a3556b5 100644 --- a/go/core/pkg/migrations/vector/000001_initial.sql +++ b/go/core/pkg/migrations/vector/000001_initial.sql @@ -18,7 +18,7 @@ CREATE INDEX idx_memory_expires_at ON memory(expires_at); -- +goose StatementBegin DO $vector$ DECLARE - vector_schema text := COALESCE(NULLIF(current_setting('kagent.vector_schema', true), ''), 'public'); + 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); 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 8713aaa8fd..fc34ea83e3 100644 --- a/helm/README.md +++ b/helm/README.md @@ -24,12 +24,13 @@ helm install kagent ./helm/kagent/ --namespace kagent --set providers.default=az ### Substrate PostgreSQL The default install uses one PostgreSQL instance and one `kagent` database. -Kagent uses the `public` schema by default. Substrate uses the `substrate` schema. +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 `public`). All Kagent installs +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 and grant the application role +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 diff --git a/helm/kagent/tests/controller-deployment_test.yaml b/helm/kagent/tests/controller-deployment_test.yaml index 2ca7b74869..762570f834 100644 --- a/helm/kagent/tests/controller-deployment_test.yaml +++ b/helm/kagent/tests/controller-deployment_test.yaml @@ -759,12 +759,12 @@ tests: path: data.POSTGRES_DATABASE_ROLE value: tenant_owner - - it: should use the public PostgreSQL schema by default + - it: should use the kagent PostgreSQL schema by default template: controller-configmap.yaml asserts: - equal: path: data.POSTGRES_DATABASE_SCHEMA - value: public + value: kagent - it: should allow a custom PostgreSQL schema template: controller-configmap.yaml @@ -775,15 +775,22 @@ tests: path: data.POSTGRES_DATABASE_SCHEMA value: kagent_custom - - it: should configure the pgvector extension schema + - it: should use the extensions schema for pgvector by default template: controller-configmap.yaml - set: - database.postgres.vectorSchema: extensions 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: diff --git a/helm/kagent/values.yaml b/helm/kagent/values.yaml index 41a4be9045..977bf5b3fc 100644 --- a/helm/kagent/values.yaml +++ b/helm/kagent/values.yaml @@ -117,10 +117,10 @@ database: # install sharing a database; bundled bootstrap requires kagent_owner. role: kagent_owner # -- Schema for Kagent tables. - schema: public + 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: public + 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. From 7af90670a95d4409de9d561c01b5b7d3822de2c6 Mon Sep 17 00:00:00 2001 From: Jeremy Alvis Date: Wed, 23 Sep 2026 10:19:21 -0700 Subject: [PATCH 11/12] Use consts for default db schemas Signed-off-by: Jeremy Alvis --- go/core/internal/database/client_postgres.go | 3 ++- go/core/internal/database/connect.go | 3 ++- go/core/pkg/consts/postgres.go | 6 ++++++ go/core/pkg/env/kagent.go | 6 ++++-- go/core/pkg/migrations/runner.go | 7 ++++--- 5 files changed, 18 insertions(+), 7 deletions(-) create mode 100644 go/core/pkg/consts/postgres.go diff --git a/go/core/internal/database/client_postgres.go b/go/core/internal/database/client_postgres.go index f328bd6170..c834b14db5 100644 --- a/go/core/internal/database/client_postgres.go +++ b/go/core/internal/database/client_postgres.go @@ -7,6 +7,7 @@ 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 @@ -19,7 +20,7 @@ type Client struct { // NewClient wraps an existing PostgreSQL pool without connecting or migrating. The caller // owns the pool and must close it. The optional pgvector schema defaults to extensions. func NewClient(db *pgxpool.Pool, vectorSchema ...string) *Client { - schema := "extensions" + schema := consts.DefaultPgvectorSchema if len(vectorSchema) > 0 && vectorSchema[0] != "" { schema = vectorSchema[0] } diff --git a/go/core/internal/database/connect.go b/go/core/internal/database/connect.go index 84c9941704..db6d056549 100644 --- a/go/core/internal/database/connect.go +++ b/go/core/internal/database/connect.go @@ -13,6 +13,7 @@ import ( "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" ) @@ -132,7 +133,7 @@ func poolConfig(cfg *PostgresConfig) (*pgxpool.Config, error) { } vectorSchema := cfg.VectorSchema if vectorSchema == "" { - vectorSchema = "extensions" + vectorSchema = consts.DefaultPgvectorSchema } if cfg.VectorEnabled && cfg.Schema == "" { return nil, errors.New("database schema is required when pgvector is enabled") 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 b9941acdde..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( @@ -147,14 +149,14 @@ var ( DatabaseSchema = RegisterStringVar( "POSTGRES_DATABASE_SCHEMA", - "kagent", + consts.DefaultPostgresTableSchema, "PostgreSQL schema for Kagent tables.", ComponentDatabase, ) DatabaseVectorSchema = RegisterStringVar( "POSTGRES_VECTOR_SCHEMA", - "extensions", + consts.DefaultPgvectorSchema, "Schema where the shared pgvector extension is installed.", ComponentDatabase, ) diff --git a/go/core/pkg/migrations/runner.go b/go/core/pkg/migrations/runner.go index ea9b23294e..70e97358c1 100644 --- a/go/core/pkg/migrations/runner.go +++ b/go/core/pkg/migrations/runner.go @@ -16,6 +16,7 @@ import ( "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" ) @@ -44,17 +45,17 @@ type Source struct { // BuiltinSources returns the built-in migration sources in the default schemas. func BuiltinSources(vectorEnabled bool) []Source { - return BuiltinSourcesInSchema(vectorEnabled, "kagent", "extensions") + 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 = "kagent" + schema = consts.DefaultPostgresTableSchema } if vectorSchema == "" { - vectorSchema = "extensions" + vectorSchema = consts.DefaultPgvectorSchema } sources := []Source{{ Name: "core", From 417584f666fac617d8c9dbd48700d7a22ed79540 Mon Sep 17 00:00:00 2001 From: Jeremy Alvis Date: Fri, 25 Sep 2026 15:17:10 -0700 Subject: [PATCH 12/12] Align PostgreSQL tools with application configuration Signed-off-by: Jeremy Alvis --- go/core/cli/internal/commands/db/db.go | 6 +++--- go/core/internal/database/bootstrap.go | 5 ++++- go/core/internal/database/bootstrap_test.go | 8 ++++++-- go/core/internal/database/connect.go | 3 --- go/core/internal/database/connect_test.go | 2 +- go/core/pkg/migrations/runner.go | 14 ++++++++------ go/core/pkg/migrations/runner_test.go | 14 ++++++++++++-- 7 files changed, 34 insertions(+), 18 deletions(-) diff --git a/go/core/cli/internal/commands/db/db.go b/go/core/cli/internal/commands/db/db.go index 76054f5379..ee1f751a21 100644 --- a/go/core/cli/internal/commands/db/db.go +++ b/go/core/cli/internal/commands/db/db.go @@ -48,14 +48,14 @@ func NewDBCmd() *cobra.Command { // precedence, on: the DATABASE_VECTOR_ENABLED env var in the CLI's own // environment (explicit operator intent, works without a cluster), the // controller's configmap on the live cluster (the same value the server -// reads), and finally the controller's default (enabled). +// reads), and finally the controller's default (disabled). func migrationSources(namespace *string) dbmigrate.SourcesFunc { return func(ctx context.Context) ([]migrations.Source, error) { - vectorEnabled := true + vectorEnabled := false if v := os.Getenv(vectorEnabledKey); v != "" { b, err := strconv.ParseBool(v) if err != nil { - fmt.Fprintf(os.Stderr, "warning: invalid %s=%q; assuming true\n", vectorEnabledKey, v) + fmt.Fprintf(os.Stderr, "warning: invalid %s=%q; assuming false\n", vectorEnabledKey, v) } else { vectorEnabled = b } diff --git a/go/core/internal/database/bootstrap.go b/go/core/internal/database/bootstrap.go index a5cb9879d4..84f5f08720 100644 --- a/go/core/internal/database/bootstrap.go +++ b/go/core/internal/database/bootstrap.go @@ -6,6 +6,7 @@ import ( "fmt" "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgxpool" "github.com/kagent-dev/kagent/go/core/pkg/migrations" ) @@ -48,10 +49,12 @@ func Bootstrap(ctx context.Context, cfg BootstrapConfig) error { if err != nil { return err } - connConfig, err := pgx.ParseConfig(dsn) + // The application DSN may contain pool-only options that PostgreSQL cannot accept. + poolConfig, err := pgxpool.ParseConfig(dsn) if err != nil { return errors.New("parse PostgreSQL bootstrap connection string: invalid value") } + connConfig := poolConfig.ConnConfig if connConfig.User != UserName { return fmt.Errorf("PostgreSQL bootstrap connection string must contain the %q user", UserName) } diff --git a/go/core/internal/database/bootstrap_test.go b/go/core/internal/database/bootstrap_test.go index c638daac58..d68279c908 100644 --- a/go/core/internal/database/bootstrap_test.go +++ b/go/core/internal/database/bootstrap_test.go @@ -22,8 +22,12 @@ func TestBootstrapCreatesManagedIdentity(t *testing.T) { dsn, err := url.Parse(sharedConnStr) require.NoError(t, err) dsn.User = url.UserPassword(UserName, UserPassword) + bootstrapDSN := *dsn + query := bootstrapDSN.Query() + query.Set("pool_max_conns", "4") + bootstrapDSN.RawQuery = query.Encode() cfg := BootstrapConfig{ - EndpointSource: dsn.String(), + EndpointSource: bootstrapDSN.String(), AdminUsername: adminConfig.User, AdminPassword: adminConfig.Password, Schema: schema, @@ -50,7 +54,7 @@ func TestBootstrapCreatesManagedIdentity(t *testing.T) { _, 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(), + EndpointSource: bootstrapDSN.String(), AdminUsername: adminConfig.User, AdminPassword: adminConfig.Password, Schema: schema, diff --git a/go/core/internal/database/connect.go b/go/core/internal/database/connect.go index db6d056549..8de25c1fff 100644 --- a/go/core/internal/database/connect.go +++ b/go/core/internal/database/connect.go @@ -141,9 +141,6 @@ func poolConfig(cfg *PostgresConfig) (*pgxpool.Config, error) { 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 } diff --git a/go/core/internal/database/connect_test.go b/go/core/internal/database/connect_test.go index 0e662f5d8c..23c40b23a8 100644 --- a/go/core/internal/database/connect_test.go +++ b/go/core/internal/database/connect_test.go @@ -132,7 +132,7 @@ func TestPoolConfigSetsSchema(t *testing.T) { Schema: "kagent", }) require.NoError(t, err) - assert.Equal(t, `"kagent", public`, config.ConnConfig.RuntimeParams["search_path"]) + assert.Equal(t, `"kagent"`, config.ConnConfig.RuntimeParams["search_path"]) } func TestPoolConfigRejectsMissingRuntimeSchema(t *testing.T) { diff --git a/go/core/pkg/migrations/runner.go b/go/core/pkg/migrations/runner.go index 70e97358c1..cb433c15d3 100644 --- a/go/core/pkg/migrations/runner.go +++ b/go/core/pkg/migrations/runner.go @@ -15,6 +15,7 @@ import ( "strings" "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgxpool" "github.com/jackc/pgx/v5/stdlib" "github.com/kagent-dev/kagent/go/core/pkg/consts" "github.com/pressly/goose/v3" @@ -474,14 +475,15 @@ func checkResolvedSchemaCollisions(ctx context.Context, url, role string, source } func openDB(url, role string) (*sql.DB, error) { - if role == "" { - return sql.Open("pgx", url) - } - config, err := pgx.ParseConfig(url) + // Migrations share the application's DSN, including any pool-only options. + poolConfig, err := pgxpool.ParseConfig(url) if err != nil { return nil, errors.New("invalid PostgreSQL connection string") } - return stdlib.OpenDB(*config, stdlib.OptionAfterConnect(func(ctx context.Context, conn *pgx.Conn) error { + if role == "" { + return stdlib.OpenDB(*poolConfig.ConnConfig), nil + } + return stdlib.OpenDB(*poolConfig.ConnConfig, 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) } @@ -491,7 +493,7 @@ func openDB(url, role string) (*sql.DB, error) { func pgvectorPreCheck(expectedSchema string) func(string) error { return func(url string) error { - db, err := sql.Open("pgx", url) + db, err := openDB(url, "") if err != nil { return fmt.Errorf("open database: %w", err) } diff --git a/go/core/pkg/migrations/runner_test.go b/go/core/pkg/migrations/runner_test.go index 38a0fe7fe6..426b52b9a2 100644 --- a/go/core/pkg/migrations/runner_test.go +++ b/go/core/pkg/migrations/runner_test.go @@ -214,6 +214,9 @@ func TestRunUpAsStableRole(t *testing.T) { t.Fatal(err) } loginURL.User = url.UserPassword("kagent_login", "rotating-password") + query := loginURL.Query() + query.Set("pool_max_conns", "4") + loginURL.RawQuery = query.Encode() if err := RunUpAsRole(t.Context(), loginURL.String(), "kagent_app", []Source{testSource(twoMigrationFS)}); err != nil { t.Fatal(err) } @@ -239,10 +242,17 @@ 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 { + migrationURL, err := url.Parse(dsn) + if err != nil { + t.Fatal(err) + } + query := migrationURL.Query() + query.Set("pool_max_conns", "4") + migrationURL.RawQuery = query.Encode() + if err := RunUp(t.Context(), migrationURL.String(), sources); err != nil { t.Fatal(err) } - if err := VerifyMigrated(t.Context(), dsn, sources); err != nil { + if err := VerifyMigrated(t.Context(), migrationURL.String(), sources); err != nil { t.Fatal(err) } for _, table := range []string{"memory", coreTrackingTable, vectorTrackingTable} {