From f0e15aacbb13b20a6b3c8020ad80da171e9c01af Mon Sep 17 00:00:00 2001 From: Lukas Piwowarski Date: Wed, 8 Jul 2026 16:08:34 +0200 Subject: [PATCH 1/2] 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 --- .../controller/assets/postgres_bootstrap.sh | 36 +++++----- .../controller/assets/postgres_bootstrap.sql | 29 ++++++++ internal/controller/common.go | 15 ++++ internal/controller/constants.go | 27 +++++-- internal/controller/lcore_config.go | 12 ++-- internal/controller/lcore_deployment.go | 47 ++++++++---- internal/controller/llama_stack_config.go | 7 +- internal/controller/postgres_deployment.go | 71 +++++++++---------- internal/controller/postgres_reconciler.go | 20 ++++-- .../lightspeed-stack-update.yaml | 12 ++-- .../expected-configs/lightspeed-stack.yaml | 12 ++-- .../expected-configs/ogx_config-update.yaml | 5 +- .../common/expected-configs/ogx_config.yaml | 5 +- .../assert-openstack-lightspeed-instance.yaml | 20 +++++- .../08-assert-openstacklightspeed-update.yaml | 20 +++++- 15 files changed, 229 insertions(+), 109 deletions(-) create mode 100644 internal/controller/assets/postgres_bootstrap.sql diff --git a/internal/controller/assets/postgres_bootstrap.sh b/internal/controller/assets/postgres_bootstrap.sh index 80ae74b..2a0fc02 100644 --- a/internal/controller/assets/postgres_bootstrap.sh +++ b/internal/controller/assets/postgres_bootstrap.sh @@ -1,25 +1,23 @@ #!/bin/bash - +# This script along with the postgres_bootstrap.sql script is responsible for preparing +# the database for consumption by the lightspeed-stack and llama-stack (OGX) apps. +# Things worth mentioning: +# +# * This script explicitly creates database for llama-stack (OGX). Database creation +# for lightspeed-stack is handled by the builtin scripts in the container image +# (it reads POSTGRESQL_DATABASE value): +# - POSTGRESQL_DATABASE: database used by lightspeed-stack +# - POSTGRESQL_LLAMA_STACK_DATABASE: database used by llama-stack (explicitly +# created by this script) set -e cat /var/lib/pgsql/data/userdata/postgresql.conf -echo "attempting to create llama-stack database and pg_trgm extension if they do not exist" - -_psql () { psql --set ON_ERROR_STOP=1 "$@" ; } - -# Create database for llama-stack conversation storage -DB_NAME="llamastack" - -echo "SELECT 'CREATE DATABASE $DB_NAME' WHERE NOT EXISTS (SELECT FROM pg_database WHERE datname = '$DB_NAME')\gexec" | _psql -d "$POSTGRESQL_DATABASE" - -# Create pg_trgm extension in default database (for OpenStack Lightspeed conversation cache) -echo "CREATE EXTENSION IF NOT EXISTS pg_trgm;" | _psql -d "$POSTGRESQL_DATABASE" - -# Create pg_trgm extension in llama-stack database (for text search if needed) -echo "CREATE EXTENSION IF NOT EXISTS pg_trgm;" | _psql -d $DB_NAME +echo "Bootstrapping PostgreSQL databases and permissions" -# Create schemas for isolating different components' data -echo "CREATE SCHEMA IF NOT EXISTS lcore;" | _psql -d "$POSTGRESQL_DATABASE" -echo "CREATE SCHEMA IF NOT EXISTS quota;" | _psql -d "$POSTGRESQL_DATABASE" -echo "CREATE SCHEMA IF NOT EXISTS conversation_cache;" | _psql -d "$POSTGRESQL_DATABASE" +psql \ + -v ON_ERROR_STOP=1 \ + -v postgresql_user="$POSTGRESQL_USER" \ + -v postgresql_lightspeed_stack_database="$POSTGRESQL_DATABASE" \ + -v postgresql_llama_stack_database="$POSTGRESQL_LLAMA_STACK_DATABASE" \ + -f "$POSTGRESQL_BOOTSTRAP_SQL_FILE" diff --git a/internal/controller/assets/postgres_bootstrap.sql b/internal/controller/assets/postgres_bootstrap.sql new file mode 100644 index 0000000..d97175b --- /dev/null +++ b/internal/controller/assets/postgres_bootstrap.sql @@ -0,0 +1,29 @@ +-- 1) LIGHTSPEED STACK DATABASE CONFIGURATION --------------------------------- +\c :postgresql_lightspeed_stack_database + +-- PostgreSQL 15+ removed the default CREATE/USAGE grants on the public schema. +-- Therefore we have to explicitly grant the permissions to the user. +GRANT USAGE, CREATE ON SCHEMA public TO :"postgresql_user"; + +-- lightspeed-stack requires rights to create additional schemas under its database +GRANT CREATE ON DATABASE :"postgresql_lightspeed_stack_database" TO :"postgresql_user"; + +-- pg_trgm is the trigram similarity extension for PostgreSQL (enables e.g. fuzzy text search) +CREATE EXTENSION IF NOT EXISTS pg_trgm; +------------------------------------------------------------------------------- + +-- 2) LLAMA STACK DATABASE CONFIGURATION -------------------------------------- +-- Create postgresql_llama_stack_database. +SELECT format('CREATE DATABASE %I', :'postgresql_llama_stack_database') +WHERE NOT EXISTS (SELECT FROM pg_database WHERE datname = :'postgresql_llama_stack_database')\gexec + +-- Connect and configure postgresql_llama_stack_database +\c :postgresql_llama_stack_database + +-- PostgreSQL 15+ removed the default CREATE/USAGE grants on the public schema. +-- Therefore we have to explicitly grant the permissions to the user. +GRANT USAGE, CREATE ON SCHEMA public TO :"postgresql_user"; + +-- pg_trgm is the trigram similarity extension for PostgreSQL (enables e.g. fuzzy text search) +CREATE EXTENSION IF NOT EXISTS pg_trgm; +------------------------------------------------------------------------------- diff --git a/internal/controller/common.go b/internal/controller/common.go index 907ce43..7ef8042 100644 --- a/internal/controller/common.go +++ b/internal/controller/common.go @@ -82,6 +82,21 @@ func getConfigMapResourceVersion(ctx context.Context, h *common_helper.Helper, n return cm.ResourceVersion, nil } +// getSecretResourceVersion retrieves the resource version of a Secret. +func getSecretResourceVersion(ctx context.Context, h *common_helper.Helper, name string, namespace string) (string, error) { + rawClient, err := getRawClient(h) + if err != nil { + return "", fmt.Errorf("failed to get raw client: %w", err) + } + + secret := &corev1.Secret{} + err = rawClient.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, secret) + if err != nil { + return "", fmt.Errorf("failed to get secret %s: %w", name, err) + } + return secret.ResourceVersion, nil +} + // providerNameToEnvVarName converts a provider name to a valid environment variable name. // It uppercases the string and replaces hyphens and dots with underscores. func providerNameToEnvVarName(providerName string) string { diff --git a/internal/controller/constants.go b/internal/controller/constants.go index 088d7ff..8aa65ac 100644 --- a/internal/controller/constants.go +++ b/internal/controller/constants.go @@ -50,14 +50,17 @@ const ( PostgresConfigMapName = "lightspeed-postgres-conf" PostgresNetworkPolicyName = "lightspeed-postgres-server" PostgresServicePort = int32(5432) - PostgresDefaultUser = "postgres" - PostgresDefaultDbName = "postgres" + PostgresLightspeedStackDbName = "lightspeed-stack" + PostgresLlamaStackDbName = "llamastack" PostgresSharedBuffers = "256MB" PostgresMaxConnections = 100 OpenStackLightspeedComponentPasswordFileName = "password" - PostgresExtensionScript = "create-extensions.sh" + PostgresUsernameSecretKey = "username" + PostgresBootstrapScript = "postgres_bootstrap.sh" + PostgresBootstrapSQLScript = "postgres_bootstrap.sql" PostgresConfigKey = "postgresql.conf.sample" - PostgresBootstrapVolumeMountPath = "/usr/share/container-scripts/postgresql/start/create-extensions.sh" + PostgresBootstrapVolumeMountPath = "/usr/share/container-scripts/postgresql/start/postgres_bootstrap.sh" + PostgresBootstrapSQLVolumeMountPath = "/usr/share/container-scripts/postgresql/start/postgres_bootstrap.sql" PostgresConfigVolumeMountPath = "/usr/share/pgsql/postgresql.conf.sample" PostgresDataVolume = "postgres-data" PostgresDataVolumeMountPath = "/var/lib/pgsql" @@ -65,8 +68,14 @@ const ( PostgresDataPVCDefaultSize = "1Gi" PostgresVarRunVolumeName = "lightspeed-postgres-var-run" PostgresVarRunVolumeMountPath = "/var/run/postgresql" - TmpVolumeName = "tmp-writable-volume" - TmpVolumeMountPath = "/tmp" + + // Non-admin user that should be usef by lightspeed-stack and llama-stack (OGX) to access + // the PostgreSQL database. This user gets created by the PostgreSQL container by setting + // the POSTGRESQL_USER and POSTGRESQL_PASSWORD environment variable. + PostgresSQLUsername = "lightspeed-app-user" + + TmpVolumeName = "tmp-writable-volume" + TmpVolumeMountPath = "/tmp" // Health probe settings for the PostgreSQL container. // Startup probe allows up to 300s for initialization (e.g. WAL recovery). // Liveness uses a longer period/timeout to avoid killing a busy-but-healthy instance. @@ -231,6 +240,7 @@ const ( // By recording the resource version of a ConfigMap in a Deployment, StatefulSet, or similar resource, // changes to the referenced ConfigMaps can be detected and trigger rollouts or reconciliation in the operator. PostgresConfigMapResourceVersionAnnotation = "ols.openshift.io/postgres-configmap-version" + PostgresSecretResourceVersionAnnotation = "ols.openshift.io/postgres-secret-version" VectorDBScriptsConfigMapVersionAnnotation = "ols.openshift.io/vector-db-scripts-configmap-version" LlamaStackConfigMapResourceVersionAnnotation = "ols.openshift.io/llamastack-configmap-version" LCoreConfigMapResourceVersionAnnotation = "ols.openshift.io/lcore-configmap-version" @@ -314,6 +324,11 @@ const ( //go:embed assets/postgres_bootstrap.sh var PostgresBootStrapScriptContent string +// PostgreSQL Bootstrap SQL - creates database, extensions, and schemas +// +//go:embed assets/postgres_bootstrap.sql +var PostgresBootStrapSQLContent string + // PostgreSQL Configuration - SSL and TLS settings // //go:embed assets/postgres.conf diff --git a/internal/controller/lcore_config.go b/internal/controller/lcore_config.go index 09f642f..4462302 100644 --- a/internal/controller/lcore_config.go +++ b/internal/controller/lcore_config.go @@ -131,14 +131,14 @@ func buildLCoreDatabaseConfig(h *common_helper.Helper, _ *apiv1beta1.OpenStackLi "postgres": map[string]interface{}{ "host": PostgresServiceName + "." + h.GetBeforeObject().GetNamespace() + ".svc", "port": PostgresServicePort, - "db": PostgresDefaultDbName, - "user": PostgresDefaultUser, + "db": PostgresLightspeedStackDbName, + "user": "${env.POSTGRESQL_USER}", "ssl_mode": PostgresDefaultSSLMode, "gss_encmode": "disable", "ca_cert_path": CABundleMountPath, // Environment variable substitution via llama_stack.core.stack.replace_env_vars - "password": "${env.POSTGRES_PASSWORD}", + "password": "${env.POSTGRESQL_PASSWORD}", // Separate schema for LCore to avoid conflicts with App Server "namespace": "lcore", @@ -163,9 +163,9 @@ func buildLCoreConversationCacheConfig(h *common_helper.Helper, _ *apiv1beta1.Op "postgres": map[string]interface{}{ "host": PostgresServiceName + "." + h.GetBeforeObject().GetNamespace() + ".svc", "port": PostgresServicePort, - "db": PostgresDefaultDbName, - "user": PostgresDefaultUser, - "password": "${env.POSTGRES_PASSWORD}", + "db": PostgresLightspeedStackDbName, + "user": "${env.POSTGRESQL_USER}", + "password": "${env.POSTGRESQL_PASSWORD}", "ssl_mode": PostgresDefaultSSLMode, "gss_encmode": "disable", "ca_cert_path": CABundleMountPath, diff --git a/internal/controller/lcore_deployment.go b/internal/controller/lcore_deployment.go index 26cd48c..8a4a78b 100644 --- a/internal/controller/lcore_deployment.go +++ b/internal/controller/lcore_deployment.go @@ -575,8 +575,9 @@ func buildLlamaStackEnvVars(h *common_helper.Helper, ctx context.Context, instan } } - // Postgres password for ${env.POSTGRES_PASSWORD} substitution in llama-stack config - envVars = append(envVars, buildPostgresPasswordEnvVar()) + // Postgres credentials for ${env.POSTGRESQL_PASSWORD} and ${env.POSTGRESQL_USER} + // substitution in llama-stack config + envVars = append(envVars, buildPostgresCredsEnvVars()...) // PostgreSQL SSL configuration for OGX (llama-stack). // 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 return envVars, nil } -// buildPostgresPasswordEnvVar returns the POSTGRES_PASSWORD env var sourced from the postgres secret. -func buildPostgresPasswordEnvVar() corev1.EnvVar { - return corev1.EnvVar{ - Name: "POSTGRES_PASSWORD", - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: PostgresSecretName, +// buildPostgresCredsEnvVars returns the POSTGRES_PASSWORD env var sourced from the postgres secret. +func buildPostgresCredsEnvVars() []corev1.EnvVar { + return []corev1.EnvVar{ + { + Name: "POSTGRESQL_PASSWORD", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: PostgresSecretName, + }, + Key: OpenStackLightspeedComponentPasswordFileName, + }, + }, + }, + { + Name: "POSTGRESQL_USER", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: PostgresSecretName, + }, + Key: PostgresUsernameSecretKey, }, - Key: OpenStackLightspeedComponentPasswordFileName, }, }, } @@ -637,12 +651,12 @@ func buildLightspeedStackEnvVars(instance *apiv1beta1.OpenStackLightspeed) []cor Name: "LIGHTSPEED_STACK_LOG_LEVEL", Value: instance.Spec.Logging.LightspeedStackLogLevel, }, - buildPostgresPasswordEnvVar(), } envVars = append(envVars, corev1.EnvVar{ Name: "RH_SERVER_OKP", Value: fmt.Sprintf("http://%s.%s.svc:%d", OKPServiceName, instance.GetNamespace(), OKPServicePort), }) + envVars = append(envVars, buildPostgresCredsEnvVars()...) return envVars } @@ -754,5 +768,14 @@ func buildConfigMapAnnotations(h *common_helper.Helper, ctx context.Context) (ma annotations[CABundleConfigMapVersionAnnotation] = caBundleVersion } + postgresSecretVersion, err := getSecretResourceVersion(ctx, h, PostgresSecretName, h.GetBeforeObject().GetNamespace()) + if err != nil { + if !errors.IsNotFound(err) { + return nil, fmt.Errorf("failed to get postgres secret resource version: %w", err) + } + } else { + annotations[PostgresSecretResourceVersionAnnotation] = postgresSecretVersion + } + return annotations, nil } diff --git a/internal/controller/llama_stack_config.go b/internal/controller/llama_stack_config.go index 5f90819..be8c3c6 100644 --- a/internal/controller/llama_stack_config.go +++ b/internal/controller/llama_stack_config.go @@ -321,10 +321,9 @@ func buildLlamaStackStorage(_ *common_helper.Helper, instance *apiv1beta1.OpenSt "type": "sql_postgres", "host": fmt.Sprintf("lightspeed-postgres-server.%s.svc", instance.GetNamespace()), "port": PostgresServicePort, - "user": "postgres", - "password": "${env.POSTGRES_PASSWORD}", - // Note: Database name is HARDCODED to "llamastack" in llama-stack's postgres adapter - // Not configurable - llama-stack ignores image_name for database selection + "user": "${env.POSTGRESQL_USER}", + "password": "${env.POSTGRESQL_PASSWORD}", + "db": PostgresLlamaStackDbName, }, } diff --git a/internal/controller/postgres_deployment.go b/internal/controller/postgres_deployment.go index 1c51f8b..825c486 100644 --- a/internal/controller/postgres_deployment.go +++ b/internal/controller/postgres_deployment.go @@ -63,7 +63,13 @@ func buildPostgresPodTemplateSpec() corev1.PodTemplateSpec { volumeMounts = append(volumeMounts, corev1.VolumeMount{ Name: "secret-" + PostgresBootstrapSecretName, MountPath: PostgresBootstrapVolumeMountPath, - SubPath: PostgresExtensionScript, + SubPath: PostgresBootstrapScript, + ReadOnly: true, + }) + volumeMounts = append(volumeMounts, corev1.VolumeMount{ + Name: "secret-" + PostgresBootstrapSecretName, + MountPath: PostgresBootstrapSQLVolumeMountPath, + SubPath: PostgresBootstrapSQLScript, ReadOnly: true, }) @@ -120,6 +126,30 @@ func buildPostgresPodTemplateSpec() corev1.PodTemplateSpec { MountPath: TmpVolumeMountPath, }) + envVars := []corev1.EnvVar{ + { + Name: "POSTGRESQL_DATABASE", + Value: PostgresLightspeedStackDbName, + }, + { + Name: "POSTGRESQL_LLAMA_STACK_DATABASE", + Value: PostgresLlamaStackDbName, + }, + { + Name: "POSTGRESQL_SHARED_BUFFERS", + Value: PostgresSharedBuffers, + }, + { + Name: "POSTGRESQL_MAX_CONNECTIONS", + Value: strconv.Itoa(PostgresMaxConnections), + }, + { + Name: "POSTGRESQL_BOOTSTRAP_SQL_FILE", + Value: PostgresBootstrapSQLVolumeMountPath, + }, + } + envVars = append(envVars, buildPostgresCredsEnvVars()...) + return corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: generatePostgresSelectorLabels(), @@ -156,42 +186,7 @@ func buildPostgresPodTemplateSpec() corev1.PodTemplateSpec { corev1.ResourceMemory: resource.MustParse("2Gi"), }, }, - Env: []corev1.EnvVar{ - { - Name: "POSTGRESQL_USER", - Value: PostgresDefaultUser, - }, - { - Name: "POSTGRESQL_DATABASE", - Value: PostgresDefaultDbName, - }, - { - Name: "POSTGRESQL_SHARED_BUFFERS", - Value: PostgresSharedBuffers, - }, - { - Name: "POSTGRESQL_MAX_CONNECTIONS", - Value: strconv.Itoa(PostgresMaxConnections), - }, - { - Name: "POSTGRESQL_ADMIN_PASSWORD", - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: PostgresSecretName}, - Key: OpenStackLightspeedComponentPasswordFileName, - }, - }, - }, - { - Name: "POSTGRESQL_PASSWORD", - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: PostgresSecretName}, - Key: OpenStackLightspeedComponentPasswordFileName, - }, - }, - }, - }, + Env: envVars, }, }, Volumes: volumes, @@ -203,7 +198,7 @@ func buildPostgresProbe(period, timeout, failure, initialDelay int32) *corev1.Pr return &corev1.Probe{ ProbeHandler: corev1.ProbeHandler{ Exec: &corev1.ExecAction{ - Command: []string{"pg_isready", "-U", PostgresDefaultUser, "-d", PostgresDefaultDbName}, + Command: []string{"/bin/sh", "-c", "pg_isready -U $POSTGRESQL_USER -d $POSTGRESQL_DATABASE"}, }, }, InitialDelaySeconds: initialDelay, diff --git a/internal/controller/postgres_reconciler.go b/internal/controller/postgres_reconciler.go index 05e74e9..7d9aff4 100644 --- a/internal/controller/postgres_reconciler.go +++ b/internal/controller/postgres_reconciler.go @@ -98,7 +98,8 @@ func reconcilePostgresBootstrapSecret(h *common_helper.Helper, ctx context.Conte result, err := controllerutil.CreateOrPatch(ctx, h.GetClient(), secret, func() error { // Set bootstrap script data secret.StringData = map[string]string{ - PostgresExtensionScript: PostgresBootStrapScriptContent, + PostgresBootstrapScript: PostgresBootStrapScriptContent, + PostgresBootstrapSQLScript: PostgresBootStrapSQLContent, } // Set owner reference return controllerutil.SetControllerReference(h.GetBeforeObject(), secret, h.GetScheme()) @@ -137,6 +138,10 @@ func reconcilePostgresSecret(h *common_helper.Helper, ctx context.Context, _ *ap } result, err := controllerutil.CreateOrPatch(ctx, h.GetClient(), secret, func() error { + if secret.Data == nil { + secret.Data = map[string][]byte{} + } + // Only set password if not already present (preserve existing password) if len(secret.Data) == 0 || secret.Data[OpenStackLightspeedComponentPasswordFileName] == nil { // Generate random password only on first creation @@ -149,7 +154,8 @@ func reconcilePostgresSecret(h *common_helper.Helper, ctx context.Context, _ *ap OpenStackLightspeedComponentPasswordFileName: []byte(encodedPassword), } } - // Set owner reference + + secret.Data[PostgresUsernameSecretKey] = []byte(PostgresSQLUsername) return controllerutil.SetControllerReference(h.GetBeforeObject(), secret, h.GetScheme()) }) @@ -299,6 +305,11 @@ func reconcilePostgresDeploymentTask(h *common_helper.Helper, ctx context.Contex return fmt.Errorf("%w: %v", ErrGetPostgresConfigMap, err) } + currentSecretVersion, err := getSecretResourceVersion(ctx, h, PostgresSecretName, h.GetBeforeObject().GetNamespace()) + if err != nil && !errors.IsNotFound(err) { + return fmt.Errorf("%w: %v", ErrGetPostgresSecret, err) + } + // Build the desired deployment pod spec podTemplateSpec := buildPostgresPodTemplateSpec() @@ -307,9 +318,10 @@ func reconcilePostgresDeploymentTask(h *common_helper.Helper, ctx context.Contex podTemplateSpec.Annotations = map[string]string{} } - // Store the current ConfigMap version in pod template annotations. - // When this changes, Kubernetes will see a pod template change and trigger a rollout. + // Store the current ConfigMap and Secret versions in pod template annotations. + // When either changes, Kubernetes will see a pod template change and trigger a rollout. podTemplateSpec.Annotations[PostgresConfigMapResourceVersionAnnotation] = currentConfigMapVersion + podTemplateSpec.Annotations[PostgresSecretResourceVersionAnnotation] = currentSecretVersion // Selective field updates (avoid update loops) replicas := int32(1) diff --git a/test/kuttl/common/expected-configs/lightspeed-stack-update.yaml b/test/kuttl/common/expected-configs/lightspeed-stack-update.yaml index e4f69ce..78dc28c 100644 --- a/test/kuttl/common/expected-configs/lightspeed-stack-update.yaml +++ b/test/kuttl/common/expected-configs/lightspeed-stack-update.yaml @@ -9,14 +9,14 @@ byok_rag: conversation_cache: postgres: ca_cert_path: /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem - db: postgres + db: lightspeed-stack gss_encmode: disable host: lightspeed-postgres-server.openstack-lightspeed.svc namespace: conversation_cache - password: ${env.POSTGRES_PASSWORD} + password: ${env.POSTGRESQL_PASSWORD} port: 5432 ssl_mode: verify-full - user: postgres + user: ${env.POSTGRESQL_USER} type: postgres customization: disable_query_system_prompt: true @@ -57,14 +57,14 @@ customization: database: postgres: ca_cert_path: /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem - db: postgres + db: lightspeed-stack gss_encmode: disable host: lightspeed-postgres-server.openstack-lightspeed.svc namespace: lcore - password: ${env.POSTGRES_PASSWORD} + password: ${env.POSTGRESQL_PASSWORD} port: 5432 ssl_mode: verify-full - user: postgres + user: ${env.POSTGRESQL_USER} inference: default_model: ibm-granite/granite-3.1-8b-instruct-UPDATE default_provider: openstack-lightspeed-provider diff --git a/test/kuttl/common/expected-configs/lightspeed-stack.yaml b/test/kuttl/common/expected-configs/lightspeed-stack.yaml index 27de495..7524fae 100644 --- a/test/kuttl/common/expected-configs/lightspeed-stack.yaml +++ b/test/kuttl/common/expected-configs/lightspeed-stack.yaml @@ -9,14 +9,14 @@ byok_rag: conversation_cache: postgres: ca_cert_path: /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem - db: postgres + db: lightspeed-stack gss_encmode: disable host: lightspeed-postgres-server.openstack-lightspeed.svc namespace: conversation_cache - password: ${env.POSTGRES_PASSWORD} + password: ${env.POSTGRESQL_PASSWORD} port: 5432 ssl_mode: verify-full - user: postgres + user: ${env.POSTGRESQL_USER} type: postgres customization: disable_query_system_prompt: true @@ -57,14 +57,14 @@ customization: database: postgres: ca_cert_path: /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem - db: postgres + db: lightspeed-stack gss_encmode: disable host: lightspeed-postgres-server.openstack-lightspeed.svc namespace: lcore - password: ${env.POSTGRES_PASSWORD} + password: ${env.POSTGRESQL_PASSWORD} port: 5432 ssl_mode: verify-full - user: postgres + user: ${env.POSTGRESQL_USER} inference: default_model: ibm-granite/granite-3.1-8b-instruct default_provider: openstack-lightspeed-provider diff --git a/test/kuttl/common/expected-configs/ogx_config-update.yaml b/test/kuttl/common/expected-configs/ogx_config-update.yaml index e84286c..3df891a 100644 --- a/test/kuttl/common/expected-configs/ogx_config-update.yaml +++ b/test/kuttl/common/expected-configs/ogx_config-update.yaml @@ -149,11 +149,12 @@ storage: db_path: /tmp/llama-stack/kv_store.db type: kv_sqlite postgres_backend: + db: llamastack host: lightspeed-postgres-server.openstack-lightspeed.svc - password: ${env.POSTGRES_PASSWORD} + password: ${env.POSTGRESQL_PASSWORD} port: 5432 type: sql_postgres - user: postgres + user: ${env.POSTGRESQL_USER} sql_default: db_path: /tmp/llama-stack/sql_store.db type: sql_sqlite diff --git a/test/kuttl/common/expected-configs/ogx_config.yaml b/test/kuttl/common/expected-configs/ogx_config.yaml index a5f7afd..e5dac1a 100644 --- a/test/kuttl/common/expected-configs/ogx_config.yaml +++ b/test/kuttl/common/expected-configs/ogx_config.yaml @@ -149,11 +149,12 @@ storage: db_path: /tmp/llama-stack/kv_store.db type: kv_sqlite postgres_backend: + db: llamastack host: lightspeed-postgres-server.openstack-lightspeed.svc - password: ${env.POSTGRES_PASSWORD} + password: ${env.POSTGRESQL_PASSWORD} port: 5432 type: sql_postgres - user: postgres + user: ${env.POSTGRESQL_USER} sql_default: db_path: /tmp/llama-stack/sql_store.db type: sql_sqlite diff --git a/test/kuttl/common/openstack-lightspeed-instance/assert-openstack-lightspeed-instance.yaml b/test/kuttl/common/openstack-lightspeed-instance/assert-openstack-lightspeed-instance.yaml index 0c52ab8..d78a395 100644 --- a/test/kuttl/common/openstack-lightspeed-instance/assert-openstack-lightspeed-instance.yaml +++ b/test/kuttl/common/openstack-lightspeed-instance/assert-openstack-lightspeed-instance.yaml @@ -31,6 +31,12 @@ metadata: namespace: openstack-lightspeed --- apiVersion: v1 +kind: Secret +metadata: + name: lightspeed-postgres-secret + namespace: openstack-lightspeed +--- +apiVersion: v1 kind: Service metadata: name: lightspeed-postgres-server @@ -219,11 +225,16 @@ spec: secretKeyRef: key: apitoken name: openstack-lightspeed-apitoken - - name: POSTGRES_PASSWORD + - name: POSTGRESQL_PASSWORD valueFrom: secretKeyRef: key: password name: lightspeed-postgres-secret + - name: POSTGRESQL_USER + valueFrom: + secretKeyRef: + key: username + name: lightspeed-postgres-secret - name: PGSSLMODE value: verify-full - name: PGSSLROOTCERT @@ -256,13 +267,18 @@ spec: env: - name: LIGHTSPEED_STACK_LOG_LEVEL value: WARNING - - name: POSTGRES_PASSWORD + - name: POSTGRESQL_PASSWORD valueFrom: secretKeyRef: key: password name: lightspeed-postgres-secret - name: RH_SERVER_OKP value: http://lightspeed-okp-server.openstack-lightspeed.svc:8080 + - name: POSTGRESQL_USER + valueFrom: + secretKeyRef: + key: username + name: lightspeed-postgres-secret volumeMounts: - name: ca-bundle mountPath: /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem diff --git a/test/kuttl/tests/update-openstacklightspeed/08-assert-openstacklightspeed-update.yaml b/test/kuttl/tests/update-openstacklightspeed/08-assert-openstacklightspeed-update.yaml index 16fd118..bfe475b 100644 --- a/test/kuttl/tests/update-openstacklightspeed/08-assert-openstacklightspeed-update.yaml +++ b/test/kuttl/tests/update-openstacklightspeed/08-assert-openstacklightspeed-update.yaml @@ -13,6 +13,12 @@ metadata: namespace: openstack-lightspeed --- apiVersion: v1 +kind: Secret +metadata: + name: lightspeed-postgres-secret + namespace: openstack-lightspeed +--- +apiVersion: v1 kind: Service metadata: name: lightspeed-postgres-server @@ -101,11 +107,16 @@ spec: name: openstack-lightspeed-apitoken-update - name: VLLM_URL value: http://mock-llm-api-server-pod-UPDATE:8000/v1 - - name: POSTGRES_PASSWORD + - name: POSTGRESQL_PASSWORD valueFrom: secretKeyRef: key: password name: lightspeed-postgres-secret + - name: POSTGRESQL_USER + valueFrom: + secretKeyRef: + key: username + name: lightspeed-postgres-secret - name: PGSSLMODE value: verify-full - name: PGSSLROOTCERT @@ -138,13 +149,18 @@ spec: env: - name: LIGHTSPEED_STACK_LOG_LEVEL value: ERROR - - name: POSTGRES_PASSWORD + - name: POSTGRESQL_PASSWORD valueFrom: secretKeyRef: key: password name: lightspeed-postgres-secret - name: RH_SERVER_OKP value: http://lightspeed-okp-server.openstack-lightspeed.svc:8080 + - name: POSTGRESQL_USER + valueFrom: + secretKeyRef: + key: username + name: lightspeed-postgres-secret volumeMounts: - name: ca-bundle mountPath: /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem From be4035fb2575f182f716af37b940f6a79951eec3 Mon Sep 17 00:00:00 2001 From: Lukas Piwowarski Date: Fri, 10 Jul 2026 10:45:43 -0400 Subject: [PATCH 2/2] Extract generateRandomString to common.go and add tests Move generateRandomString from postgres_reconciler.go to common.go so the helper is available across the package. Add tests covering output length, hex character validity, and uniqueness. --- internal/controller/common.go | 16 ++++ internal/controller/common_test.go | 100 +++++++++++++++++++++ internal/controller/postgres_reconciler.go | 16 ++-- 3 files changed, 122 insertions(+), 10 deletions(-) create mode 100644 internal/controller/common_test.go diff --git a/internal/controller/common.go b/internal/controller/common.go index 7ef8042..88c62d3 100644 --- a/internal/controller/common.go +++ b/internal/controller/common.go @@ -18,7 +18,9 @@ package controller import ( "context" + "crypto/rand" _ "embed" + "encoding/hex" "encoding/json" "errors" "fmt" @@ -189,3 +191,17 @@ func getDeployment(ctx context.Context, h *common_helper.Helper, name string, na return deployment, nil } + +// generateRandomString generates a random hex string of the given length. +func generateRandomString(secretLength int) (string, error) { + // Ceiling division: hex.EncodeToString doubles the byte count, so we need ceil(secretLength/2) bytes. + randDataLen := (secretLength + 1) / 2 + + b := make([]byte, randDataLen) + if _, err := rand.Read(b); err != nil { + return "", fmt.Errorf("failed to generate secret: %w", err) + } + + randString := hex.EncodeToString(b) + return randString[:secretLength], nil +} diff --git a/internal/controller/common_test.go b/internal/controller/common_test.go new file mode 100644 index 0000000..59860e7 --- /dev/null +++ b/internal/controller/common_test.go @@ -0,0 +1,100 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "testing" +) + +func TestGenerateRandomStringLength(t *testing.T) { + tests := []struct { + name string + length int + }{ + { + name: "Zero length", + length: 0, + }, + { + name: "Odd length 1", + length: 1, + }, + { + name: "Even length 2", + length: 2, + }, + { + name: "Odd length 7", + length: 7, + }, + { + name: "Even length 8", + length: 8, + }, + { + name: "Odd length 15", + length: 15, + }, + { + name: "Even length 16", + length: 16, + }, + { + name: "Even length 32", + length: 32, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := generateRandomString(tt.length) + if err != nil { + t.Errorf("generateRandomString(%d) unexpected error: %v", tt.length, err) + } + if len(result) != tt.length { + t.Errorf("generateRandomString(%d) returned length %d, want %d", tt.length, len(result), tt.length) + } + }) + } +} + +func TestGenerateRandomStringHexCharacters(t *testing.T) { + result, err := generateRandomString(32) + if err != nil { + t.Fatalf("generateRandomString(32) unexpected error: %v", err) + } + for i, c := range result { + if (c < '0' || c > '9') && (c < 'a' || c > 'f') { + t.Errorf("generateRandomString(32) character at index %d is %q, not a lowercase hex character", i, c) + } + } +} + +func TestGenerateRandomStringUniqueness(t *testing.T) { + const length = 16 + a, err := generateRandomString(length) + if err != nil { + t.Fatalf("first call unexpected error: %v", err) + } + b, err := generateRandomString(length) + if err != nil { + t.Fatalf("second call unexpected error: %v", err) + } + if a == b { + t.Errorf("generateRandomString(%d) returned identical values across two calls: %q", length, a) + } +} diff --git a/internal/controller/postgres_reconciler.go b/internal/controller/postgres_reconciler.go index 7d9aff4..0fb6a2e 100644 --- a/internal/controller/postgres_reconciler.go +++ b/internal/controller/postgres_reconciler.go @@ -18,8 +18,6 @@ package controller import ( "context" - "crypto/rand" - "encoding/base64" "fmt" common_helper "github.com/openstack-k8s-operators/lib-common/modules/common/helper" @@ -143,16 +141,14 @@ func reconcilePostgresSecret(h *common_helper.Helper, ctx context.Context, _ *ap } // Only set password if not already present (preserve existing password) - if len(secret.Data) == 0 || secret.Data[OpenStackLightspeedComponentPasswordFileName] == nil { - // Generate random password only on first creation - randomPassword := make([]byte, 12) - if _, err := rand.Read(randomPassword); err != nil { + if secret.Data[OpenStackLightspeedComponentPasswordFileName] == nil { + const PostgreSQLPasswordLen = 16 + password, err := generateRandomString(PostgreSQLPasswordLen) + if err != nil { return fmt.Errorf("%w: %v", ErrGeneratePostgresSecret, err) } - encodedPassword := base64.StdEncoding.EncodeToString(randomPassword) - secret.Data = map[string][]byte{ - OpenStackLightspeedComponentPasswordFileName: []byte(encodedPassword), - } + + secret.Data[OpenStackLightspeedComponentPasswordFileName] = []byte(password) } secret.Data[PostgresUsernameSecretKey] = []byte(PostgresSQLUsername)