Skip to content

Commit f0e15aa

Browse files
committed
Fix PostgreSQL credential security issues and naming inconsistencies
The previous PostgreSQL setup had several security and consistency problems: Security issues: 1. POSTGRESQL_ADMIN_PASSWORD was set, making the postgres superuser accessible remotely (not just locally via `oc rsh` + `psql`). [1] 2. Both lightspeed-stack and llama-stack were connecting as the postgres superuser rather than a dedicated non-admin user. 3. The postgres superuser username usage was hardcoded and credential paths into the app and db pods were inconsistent. Naming/consistency issues: 4. lightspeed-stack used database "postgres" while llama-stack used "llamastack", with no explicit database name in llama-stack's config. 5. POSTGRES_USER/POSTGRES_PASSWORD env var names did not follow the POSTGRESQL_* convention of the PostgreSQL container image and also the usage of these env vars was insonsistent across the operator code. 6. Bootstrap SQL was embedded in postgres_bootstrap.sh, making it hard to read and maintain. Fixes: - Remove POSTGRESQL_ADMIN_PASSWORD. The postgres superuser is now only accessible locally via `oc rsh` + `psql` (no password required). - Introduce a dedicated non-admin user "lightspeed-app-user", stored in lightspeed-postgres-secret alongside the password. Both apps now connect as this user via POSTGRESQL_USER and POSTGRESQL_PASSWORD. - Rename the lightspeed-stack database from "postgres" to "lightspeed-stack" and make llama-stack's "llamastack" database selection explicit in its config. - Rename POSTGRES_USER/POSTGRES_PASSWORD -> POSTGRESQL_USER/POSTGRESQL_PASSWORD throughout to match the container image's naming convention. - Extract bootstrap SQL into postgres_bootstrap.sql to enable syntax highlighting and improve maintainability. [1] https://github.com/sclorg/postgresql-container/blob/master/src/root/usr/share/container-scripts/postgresql/README.md#postgresql-admin-account
1 parent ed059d7 commit f0e15aa

15 files changed

Lines changed: 229 additions & 109 deletions
Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,23 @@
11
#!/bin/bash
2-
2+
# This script along with the postgres_bootstrap.sql script is responsible for preparing
3+
# the database for consumption by the lightspeed-stack and llama-stack (OGX) apps.
4+
# Things worth mentioning:
5+
#
6+
# * This script explicitly creates database for llama-stack (OGX). Database creation
7+
# for lightspeed-stack is handled by the builtin scripts in the container image
8+
# (it reads POSTGRESQL_DATABASE value):
9+
# - POSTGRESQL_DATABASE: database used by lightspeed-stack
10+
# - POSTGRESQL_LLAMA_STACK_DATABASE: database used by llama-stack (explicitly
11+
# created by this script)
312
set -e
413

514
cat /var/lib/pgsql/data/userdata/postgresql.conf
615

7-
echo "attempting to create llama-stack database and pg_trgm extension if they do not exist"
8-
9-
_psql () { psql --set ON_ERROR_STOP=1 "$@" ; }
10-
11-
# Create database for llama-stack conversation storage
12-
DB_NAME="llamastack"
13-
14-
echo "SELECT 'CREATE DATABASE $DB_NAME' WHERE NOT EXISTS (SELECT FROM pg_database WHERE datname = '$DB_NAME')\gexec" | _psql -d "$POSTGRESQL_DATABASE"
15-
16-
# Create pg_trgm extension in default database (for OpenStack Lightspeed conversation cache)
17-
echo "CREATE EXTENSION IF NOT EXISTS pg_trgm;" | _psql -d "$POSTGRESQL_DATABASE"
18-
19-
# Create pg_trgm extension in llama-stack database (for text search if needed)
20-
echo "CREATE EXTENSION IF NOT EXISTS pg_trgm;" | _psql -d $DB_NAME
16+
echo "Bootstrapping PostgreSQL databases and permissions"
2117

22-
# Create schemas for isolating different components' data
23-
echo "CREATE SCHEMA IF NOT EXISTS lcore;" | _psql -d "$POSTGRESQL_DATABASE"
24-
echo "CREATE SCHEMA IF NOT EXISTS quota;" | _psql -d "$POSTGRESQL_DATABASE"
25-
echo "CREATE SCHEMA IF NOT EXISTS conversation_cache;" | _psql -d "$POSTGRESQL_DATABASE"
18+
psql \
19+
-v ON_ERROR_STOP=1 \
20+
-v postgresql_user="$POSTGRESQL_USER" \
21+
-v postgresql_lightspeed_stack_database="$POSTGRESQL_DATABASE" \
22+
-v postgresql_llama_stack_database="$POSTGRESQL_LLAMA_STACK_DATABASE" \
23+
-f "$POSTGRESQL_BOOTSTRAP_SQL_FILE"
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
-- 1) LIGHTSPEED STACK DATABASE CONFIGURATION ---------------------------------
2+
\c :postgresql_lightspeed_stack_database
3+
4+
-- PostgreSQL 15+ removed the default CREATE/USAGE grants on the public schema.
5+
-- Therefore we have to explicitly grant the permissions to the user.
6+
GRANT USAGE, CREATE ON SCHEMA public TO :"postgresql_user";
7+
8+
-- lightspeed-stack requires rights to create additional schemas under its database
9+
GRANT CREATE ON DATABASE :"postgresql_lightspeed_stack_database" TO :"postgresql_user";
10+
11+
-- pg_trgm is the trigram similarity extension for PostgreSQL (enables e.g. fuzzy text search)
12+
CREATE EXTENSION IF NOT EXISTS pg_trgm;
13+
-------------------------------------------------------------------------------
14+
15+
-- 2) LLAMA STACK DATABASE CONFIGURATION --------------------------------------
16+
-- Create postgresql_llama_stack_database.
17+
SELECT format('CREATE DATABASE %I', :'postgresql_llama_stack_database')
18+
WHERE NOT EXISTS (SELECT FROM pg_database WHERE datname = :'postgresql_llama_stack_database')\gexec
19+
20+
-- Connect and configure postgresql_llama_stack_database
21+
\c :postgresql_llama_stack_database
22+
23+
-- PostgreSQL 15+ removed the default CREATE/USAGE grants on the public schema.
24+
-- Therefore we have to explicitly grant the permissions to the user.
25+
GRANT USAGE, CREATE ON SCHEMA public TO :"postgresql_user";
26+
27+
-- pg_trgm is the trigram similarity extension for PostgreSQL (enables e.g. fuzzy text search)
28+
CREATE EXTENSION IF NOT EXISTS pg_trgm;
29+
-------------------------------------------------------------------------------

internal/controller/common.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,21 @@ func getConfigMapResourceVersion(ctx context.Context, h *common_helper.Helper, n
8282
return cm.ResourceVersion, nil
8383
}
8484

85+
// getSecretResourceVersion retrieves the resource version of a Secret.
86+
func getSecretResourceVersion(ctx context.Context, h *common_helper.Helper, name string, namespace string) (string, error) {
87+
rawClient, err := getRawClient(h)
88+
if err != nil {
89+
return "", fmt.Errorf("failed to get raw client: %w", err)
90+
}
91+
92+
secret := &corev1.Secret{}
93+
err = rawClient.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, secret)
94+
if err != nil {
95+
return "", fmt.Errorf("failed to get secret %s: %w", name, err)
96+
}
97+
return secret.ResourceVersion, nil
98+
}
99+
85100
// providerNameToEnvVarName converts a provider name to a valid environment variable name.
86101
// It uppercases the string and replaces hyphens and dots with underscores.
87102
func providerNameToEnvVarName(providerName string) string {

internal/controller/constants.go

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,23 +50,32 @@ const (
5050
PostgresConfigMapName = "lightspeed-postgres-conf"
5151
PostgresNetworkPolicyName = "lightspeed-postgres-server"
5252
PostgresServicePort = int32(5432)
53-
PostgresDefaultUser = "postgres"
54-
PostgresDefaultDbName = "postgres"
53+
PostgresLightspeedStackDbName = "lightspeed-stack"
54+
PostgresLlamaStackDbName = "llamastack"
5555
PostgresSharedBuffers = "256MB"
5656
PostgresMaxConnections = 100
5757
OpenStackLightspeedComponentPasswordFileName = "password"
58-
PostgresExtensionScript = "create-extensions.sh"
58+
PostgresUsernameSecretKey = "username"
59+
PostgresBootstrapScript = "postgres_bootstrap.sh"
60+
PostgresBootstrapSQLScript = "postgres_bootstrap.sql"
5961
PostgresConfigKey = "postgresql.conf.sample"
60-
PostgresBootstrapVolumeMountPath = "/usr/share/container-scripts/postgresql/start/create-extensions.sh"
62+
PostgresBootstrapVolumeMountPath = "/usr/share/container-scripts/postgresql/start/postgres_bootstrap.sh"
63+
PostgresBootstrapSQLVolumeMountPath = "/usr/share/container-scripts/postgresql/start/postgres_bootstrap.sql"
6164
PostgresConfigVolumeMountPath = "/usr/share/pgsql/postgresql.conf.sample"
6265
PostgresDataVolume = "postgres-data"
6366
PostgresDataVolumeMountPath = "/var/lib/pgsql"
6467
PostgresDataPVCName = "openstack-lightspeed-database"
6568
PostgresDataPVCDefaultSize = "1Gi"
6669
PostgresVarRunVolumeName = "lightspeed-postgres-var-run"
6770
PostgresVarRunVolumeMountPath = "/var/run/postgresql"
68-
TmpVolumeName = "tmp-writable-volume"
69-
TmpVolumeMountPath = "/tmp"
71+
72+
// Non-admin user that should be usef by lightspeed-stack and llama-stack (OGX) to access
73+
// the PostgreSQL database. This user gets created by the PostgreSQL container by setting
74+
// the POSTGRESQL_USER and POSTGRESQL_PASSWORD environment variable.
75+
PostgresSQLUsername = "lightspeed-app-user"
76+
77+
TmpVolumeName = "tmp-writable-volume"
78+
TmpVolumeMountPath = "/tmp"
7079
// Health probe settings for the PostgreSQL container.
7180
// Startup probe allows up to 300s for initialization (e.g. WAL recovery).
7281
// Liveness uses a longer period/timeout to avoid killing a busy-but-healthy instance.
@@ -231,6 +240,7 @@ const (
231240
// By recording the resource version of a ConfigMap in a Deployment, StatefulSet, or similar resource,
232241
// changes to the referenced ConfigMaps can be detected and trigger rollouts or reconciliation in the operator.
233242
PostgresConfigMapResourceVersionAnnotation = "ols.openshift.io/postgres-configmap-version"
243+
PostgresSecretResourceVersionAnnotation = "ols.openshift.io/postgres-secret-version"
234244
VectorDBScriptsConfigMapVersionAnnotation = "ols.openshift.io/vector-db-scripts-configmap-version"
235245
LlamaStackConfigMapResourceVersionAnnotation = "ols.openshift.io/llamastack-configmap-version"
236246
LCoreConfigMapResourceVersionAnnotation = "ols.openshift.io/lcore-configmap-version"
@@ -314,6 +324,11 @@ const (
314324
//go:embed assets/postgres_bootstrap.sh
315325
var PostgresBootStrapScriptContent string
316326

327+
// PostgreSQL Bootstrap SQL - creates database, extensions, and schemas
328+
//
329+
//go:embed assets/postgres_bootstrap.sql
330+
var PostgresBootStrapSQLContent string
331+
317332
// PostgreSQL Configuration - SSL and TLS settings
318333
//
319334
//go:embed assets/postgres.conf

internal/controller/lcore_config.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -131,14 +131,14 @@ func buildLCoreDatabaseConfig(h *common_helper.Helper, _ *apiv1beta1.OpenStackLi
131131
"postgres": map[string]interface{}{
132132
"host": PostgresServiceName + "." + h.GetBeforeObject().GetNamespace() + ".svc",
133133
"port": PostgresServicePort,
134-
"db": PostgresDefaultDbName,
135-
"user": PostgresDefaultUser,
134+
"db": PostgresLightspeedStackDbName,
135+
"user": "${env.POSTGRESQL_USER}",
136136
"ssl_mode": PostgresDefaultSSLMode,
137137
"gss_encmode": "disable",
138138
"ca_cert_path": CABundleMountPath,
139139

140140
// Environment variable substitution via llama_stack.core.stack.replace_env_vars
141-
"password": "${env.POSTGRES_PASSWORD}",
141+
"password": "${env.POSTGRESQL_PASSWORD}",
142142

143143
// Separate schema for LCore to avoid conflicts with App Server
144144
"namespace": "lcore",
@@ -163,9 +163,9 @@ func buildLCoreConversationCacheConfig(h *common_helper.Helper, _ *apiv1beta1.Op
163163
"postgres": map[string]interface{}{
164164
"host": PostgresServiceName + "." + h.GetBeforeObject().GetNamespace() + ".svc",
165165
"port": PostgresServicePort,
166-
"db": PostgresDefaultDbName,
167-
"user": PostgresDefaultUser,
168-
"password": "${env.POSTGRES_PASSWORD}",
166+
"db": PostgresLightspeedStackDbName,
167+
"user": "${env.POSTGRESQL_USER}",
168+
"password": "${env.POSTGRESQL_PASSWORD}",
169169
"ssl_mode": PostgresDefaultSSLMode,
170170
"gss_encmode": "disable",
171171
"ca_cert_path": CABundleMountPath,

internal/controller/lcore_deployment.go

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -575,8 +575,9 @@ func buildLlamaStackEnvVars(h *common_helper.Helper, ctx context.Context, instan
575575
}
576576
}
577577

578-
// Postgres password for ${env.POSTGRES_PASSWORD} substitution in llama-stack config
579-
envVars = append(envVars, buildPostgresPasswordEnvVar())
578+
// Postgres credentials for ${env.POSTGRESQL_PASSWORD} and ${env.POSTGRESQL_USER}
579+
// substitution in llama-stack config
580+
envVars = append(envVars, buildPostgresCredsEnvVars()...)
580581

581582
// PostgreSQL SSL configuration for OGX (llama-stack).
582583
// OGX's PostgresSqlStoreConfig does not support ssl_mode/ca_cert_path fields yet
@@ -615,16 +616,29 @@ func buildLlamaStackEnvVars(h *common_helper.Helper, ctx context.Context, instan
615616
return envVars, nil
616617
}
617618

618-
// buildPostgresPasswordEnvVar returns the POSTGRES_PASSWORD env var sourced from the postgres secret.
619-
func buildPostgresPasswordEnvVar() corev1.EnvVar {
620-
return corev1.EnvVar{
621-
Name: "POSTGRES_PASSWORD",
622-
ValueFrom: &corev1.EnvVarSource{
623-
SecretKeyRef: &corev1.SecretKeySelector{
624-
LocalObjectReference: corev1.LocalObjectReference{
625-
Name: PostgresSecretName,
619+
// buildPostgresCredsEnvVars returns the POSTGRES_PASSWORD env var sourced from the postgres secret.
620+
func buildPostgresCredsEnvVars() []corev1.EnvVar {
621+
return []corev1.EnvVar{
622+
{
623+
Name: "POSTGRESQL_PASSWORD",
624+
ValueFrom: &corev1.EnvVarSource{
625+
SecretKeyRef: &corev1.SecretKeySelector{
626+
LocalObjectReference: corev1.LocalObjectReference{
627+
Name: PostgresSecretName,
628+
},
629+
Key: OpenStackLightspeedComponentPasswordFileName,
630+
},
631+
},
632+
},
633+
{
634+
Name: "POSTGRESQL_USER",
635+
ValueFrom: &corev1.EnvVarSource{
636+
SecretKeyRef: &corev1.SecretKeySelector{
637+
LocalObjectReference: corev1.LocalObjectReference{
638+
Name: PostgresSecretName,
639+
},
640+
Key: PostgresUsernameSecretKey,
626641
},
627-
Key: OpenStackLightspeedComponentPasswordFileName,
628642
},
629643
},
630644
}
@@ -637,12 +651,12 @@ func buildLightspeedStackEnvVars(instance *apiv1beta1.OpenStackLightspeed) []cor
637651
Name: "LIGHTSPEED_STACK_LOG_LEVEL",
638652
Value: instance.Spec.Logging.LightspeedStackLogLevel,
639653
},
640-
buildPostgresPasswordEnvVar(),
641654
}
642655
envVars = append(envVars, corev1.EnvVar{
643656
Name: "RH_SERVER_OKP",
644657
Value: fmt.Sprintf("http://%s.%s.svc:%d", OKPServiceName, instance.GetNamespace(), OKPServicePort),
645658
})
659+
envVars = append(envVars, buildPostgresCredsEnvVars()...)
646660
return envVars
647661
}
648662

@@ -754,5 +768,14 @@ func buildConfigMapAnnotations(h *common_helper.Helper, ctx context.Context) (ma
754768
annotations[CABundleConfigMapVersionAnnotation] = caBundleVersion
755769
}
756770

771+
postgresSecretVersion, err := getSecretResourceVersion(ctx, h, PostgresSecretName, h.GetBeforeObject().GetNamespace())
772+
if err != nil {
773+
if !errors.IsNotFound(err) {
774+
return nil, fmt.Errorf("failed to get postgres secret resource version: %w", err)
775+
}
776+
} else {
777+
annotations[PostgresSecretResourceVersionAnnotation] = postgresSecretVersion
778+
}
779+
757780
return annotations, nil
758781
}

internal/controller/llama_stack_config.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -321,10 +321,9 @@ func buildLlamaStackStorage(_ *common_helper.Helper, instance *apiv1beta1.OpenSt
321321
"type": "sql_postgres",
322322
"host": fmt.Sprintf("lightspeed-postgres-server.%s.svc", instance.GetNamespace()),
323323
"port": PostgresServicePort,
324-
"user": "postgres",
325-
"password": "${env.POSTGRES_PASSWORD}",
326-
// Note: Database name is HARDCODED to "llamastack" in llama-stack's postgres adapter
327-
// Not configurable - llama-stack ignores image_name for database selection
324+
"user": "${env.POSTGRESQL_USER}",
325+
"password": "${env.POSTGRESQL_PASSWORD}",
326+
"db": PostgresLlamaStackDbName,
328327
},
329328
}
330329

internal/controller/postgres_deployment.go

Lines changed: 33 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,13 @@ func buildPostgresPodTemplateSpec() corev1.PodTemplateSpec {
6363
volumeMounts = append(volumeMounts, corev1.VolumeMount{
6464
Name: "secret-" + PostgresBootstrapSecretName,
6565
MountPath: PostgresBootstrapVolumeMountPath,
66-
SubPath: PostgresExtensionScript,
66+
SubPath: PostgresBootstrapScript,
67+
ReadOnly: true,
68+
})
69+
volumeMounts = append(volumeMounts, corev1.VolumeMount{
70+
Name: "secret-" + PostgresBootstrapSecretName,
71+
MountPath: PostgresBootstrapSQLVolumeMountPath,
72+
SubPath: PostgresBootstrapSQLScript,
6773
ReadOnly: true,
6874
})
6975

@@ -120,6 +126,30 @@ func buildPostgresPodTemplateSpec() corev1.PodTemplateSpec {
120126
MountPath: TmpVolumeMountPath,
121127
})
122128

129+
envVars := []corev1.EnvVar{
130+
{
131+
Name: "POSTGRESQL_DATABASE",
132+
Value: PostgresLightspeedStackDbName,
133+
},
134+
{
135+
Name: "POSTGRESQL_LLAMA_STACK_DATABASE",
136+
Value: PostgresLlamaStackDbName,
137+
},
138+
{
139+
Name: "POSTGRESQL_SHARED_BUFFERS",
140+
Value: PostgresSharedBuffers,
141+
},
142+
{
143+
Name: "POSTGRESQL_MAX_CONNECTIONS",
144+
Value: strconv.Itoa(PostgresMaxConnections),
145+
},
146+
{
147+
Name: "POSTGRESQL_BOOTSTRAP_SQL_FILE",
148+
Value: PostgresBootstrapSQLVolumeMountPath,
149+
},
150+
}
151+
envVars = append(envVars, buildPostgresCredsEnvVars()...)
152+
123153
return corev1.PodTemplateSpec{
124154
ObjectMeta: metav1.ObjectMeta{
125155
Labels: generatePostgresSelectorLabels(),
@@ -156,42 +186,7 @@ func buildPostgresPodTemplateSpec() corev1.PodTemplateSpec {
156186
corev1.ResourceMemory: resource.MustParse("2Gi"),
157187
},
158188
},
159-
Env: []corev1.EnvVar{
160-
{
161-
Name: "POSTGRESQL_USER",
162-
Value: PostgresDefaultUser,
163-
},
164-
{
165-
Name: "POSTGRESQL_DATABASE",
166-
Value: PostgresDefaultDbName,
167-
},
168-
{
169-
Name: "POSTGRESQL_SHARED_BUFFERS",
170-
Value: PostgresSharedBuffers,
171-
},
172-
{
173-
Name: "POSTGRESQL_MAX_CONNECTIONS",
174-
Value: strconv.Itoa(PostgresMaxConnections),
175-
},
176-
{
177-
Name: "POSTGRESQL_ADMIN_PASSWORD",
178-
ValueFrom: &corev1.EnvVarSource{
179-
SecretKeyRef: &corev1.SecretKeySelector{
180-
LocalObjectReference: corev1.LocalObjectReference{Name: PostgresSecretName},
181-
Key: OpenStackLightspeedComponentPasswordFileName,
182-
},
183-
},
184-
},
185-
{
186-
Name: "POSTGRESQL_PASSWORD",
187-
ValueFrom: &corev1.EnvVarSource{
188-
SecretKeyRef: &corev1.SecretKeySelector{
189-
LocalObjectReference: corev1.LocalObjectReference{Name: PostgresSecretName},
190-
Key: OpenStackLightspeedComponentPasswordFileName,
191-
},
192-
},
193-
},
194-
},
189+
Env: envVars,
195190
},
196191
},
197192
Volumes: volumes,
@@ -203,7 +198,7 @@ func buildPostgresProbe(period, timeout, failure, initialDelay int32) *corev1.Pr
203198
return &corev1.Probe{
204199
ProbeHandler: corev1.ProbeHandler{
205200
Exec: &corev1.ExecAction{
206-
Command: []string{"pg_isready", "-U", PostgresDefaultUser, "-d", PostgresDefaultDbName},
201+
Command: []string{"/bin/sh", "-c", "pg_isready -U $POSTGRESQL_USER -d $POSTGRESQL_DATABASE"},
207202
},
208203
},
209204
InitialDelaySeconds: initialDelay,

0 commit comments

Comments
 (0)