From 475008e05d3ebc8eaabec3cfcaf7021ef81f87aa Mon Sep 17 00:00:00 2001 From: jjaruszewski Date: Tue, 2 Jun 2026 14:05:35 +0200 Subject: [PATCH 1/4] Initial global images config Signed-off-by: jjaruszewski --- cmd/postgres-operator/main.go | 10 + internal/config/config.go | 108 ++++++++- percona/config/images/config.go | 31 +++ percona/config/images/default-images.yaml | 98 +++++++++ percona/config/images/loader.go | 169 ++++++++++++++ percona/config/images/merge.go | 110 ++++++++++ percona/config/images/merge_test.go | 254 ++++++++++++++++++++++ percona/config/images/types.go | 120 ++++++++++ percona/config/images/types_test.go | 203 +++++++++++++++++ 9 files changed, 1095 insertions(+), 8 deletions(-) create mode 100644 percona/config/images/config.go create mode 100644 percona/config/images/default-images.yaml create mode 100644 percona/config/images/loader.go create mode 100644 percona/config/images/merge.go create mode 100644 percona/config/images/merge_test.go create mode 100644 percona/config/images/types.go create mode 100644 percona/config/images/types_test.go diff --git a/cmd/postgres-operator/main.go b/cmd/postgres-operator/main.go index f61123c4b..06ea6d655 100644 --- a/cmd/postgres-operator/main.go +++ b/cmd/postgres-operator/main.go @@ -38,6 +38,7 @@ import ( "github.com/percona/percona-postgresql-operator/v2/internal/naming" "github.com/percona/percona-postgresql-operator/v2/internal/upgradecheck" "github.com/percona/percona-postgresql-operator/v2/percona/certmanager" + "github.com/percona/percona-postgresql-operator/v2/percona/config/images" perconaController "github.com/percona/percona-postgresql-operator/v2/percona/controller" "github.com/percona/percona-postgresql-operator/v2/percona/controller/pgbackup" "github.com/percona/percona-postgresql-operator/v2/percona/controller/pgcluster" @@ -124,6 +125,15 @@ func main() { ) assertNoError(err) + // Initialize image configuration + log.Info("Initializing image configuration") + if err := images.InitializeGlobalConfig(ctx, mgr.GetClient()); err != nil { + log.Error(err, "Failed to initialize image configuration, using embedded defaults") + images.SetGlobalConfig(images.DefaultConfig) + } else { + log.Info("Image configuration initialized successfully") + } + // Add Percona custom resource types to scheme assertNoError(v2.AddToScheme(mgr.GetScheme())) diff --git a/internal/config/config.go b/internal/config/config.go index 5f7307909..91ada7954 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -10,6 +10,8 @@ import ( "github.com/pkg/errors" + "github.com/percona/percona-postgresql-operator/v2/percona/config/images" + pNaming "github.com/percona/percona-postgresql-operator/v2/percona/naming" "github.com/percona/percona-postgresql-operator/v2/pkg/apis/upstream.pgv2.percona.com/v1beta1" ) @@ -21,6 +23,35 @@ func defaultFromEnv(value, key string) string { return value } +// getImageFromOperatorConfig attempts to get image from operator-wide configuration +func getImageFromOperatorConfig(cluster *v1beta1.PostgresCluster, component string) string { + crVersion := getCRVersionForCluster(cluster) + if crVersion == "" { + return "" + } + + pgVersion := fmt.Sprintf("%d", cluster.Spec.PostgresVersion) + + // If PostGIS is enabled, use the postgresGIS component instead + if cluster.Spec.PostGISVersion != "" { + component = "postgresGIS" + } + + return images.GetImageForCluster(crVersion, component, pgVersion) +} + +// getCRVersionForCluster retrieves the CR version for a cluster +// The Percona operator stores the CR version in the LabelOperatorVersion label +func getCRVersionForCluster(cluster *v1beta1.PostgresCluster) string { + // Check the standard Percona version label + if cluster.Labels != nil { + if version, ok := cluster.Labels[pNaming.LabelOperatorVersion]; ok { + return version + } + } + return "" +} + // FetchKeyCommand returns the fetch_key_cmd value stored in the encryption_key_command // variable used to enable TDE. func FetchKeyCommand(spec *v1beta1.PostgresClusterSpec) string { @@ -53,7 +84,17 @@ func FetchKeyCommand(spec *v1beta1.PostgresClusterSpec) string { func PGBackRestContainerImage(cluster *v1beta1.PostgresCluster) string { image := cluster.Spec.Backups.PGBackRest.Image - return defaultFromEnv(image, "RELATED_IMAGE_PGBACKREST") + // Try operator-wide config first + if image == "" { + image = getImageFromOperatorConfig(cluster, "pgbackrest") + } + + // Fallback to env var + if image == "" { + image = defaultFromEnv(image, "RELATED_IMAGE_PGBACKREST") + } + + return image } // PGAdminContainerImage returns the container image to use for pgAdmin. @@ -64,7 +105,17 @@ func PGAdminContainerImage(cluster *v1beta1.PostgresCluster) string { image = cluster.Spec.UserInterface.PGAdmin.Image } - return defaultFromEnv(image, "RELATED_IMAGE_PGADMIN") + // Try operator-wide config first + if image == "" { + image = getImageFromOperatorConfig(cluster, "pgadmin") + } + + // Fallback to env var + if image == "" { + image = defaultFromEnv(image, "RELATED_IMAGE_PGADMIN") + } + + return image } // StandalonePGAdminContainerImage returns the container image to use for pgAdmin. @@ -74,7 +125,18 @@ func StandalonePGAdminContainerImage(pgadmin *v1beta1.PGAdmin) string { image = *pgadmin.Spec.Image } - return defaultFromEnv(image, "RELATED_IMAGE_STANDALONE_PGADMIN") + // Try operator-wide config first + if image == "" { + // For standalone pgadmin, we don't have cluster context + // Use a default version or skip + } + + // Fallback to env var + if image == "" { + image = defaultFromEnv(image, "RELATED_IMAGE_STANDALONE_PGADMIN") + } + + return image } // PGBouncerContainerImage returns the container image to use for pgBouncer. @@ -85,7 +147,17 @@ func PGBouncerContainerImage(cluster *v1beta1.PostgresCluster) string { image = cluster.Spec.Proxy.PGBouncer.Image } - return defaultFromEnv(image, "RELATED_IMAGE_PGBOUNCER") + // Try operator-wide config first + if image == "" { + image = getImageFromOperatorConfig(cluster, "pgbouncer") + } + + // Fallback to env var + if image == "" { + image = defaultFromEnv(image, "RELATED_IMAGE_PGBOUNCER") + } + + return image } // PGExporterContainerImage returns the container image to use for the @@ -98,7 +170,17 @@ func PGExporterContainerImage(cluster *v1beta1.PostgresCluster) string { image = cluster.Spec.Monitoring.PGMonitor.Exporter.Image } - return defaultFromEnv(image, "RELATED_IMAGE_PGEXPORTER") + // Try operator-wide config first + if image == "" { + image = getImageFromOperatorConfig(cluster, "pgexporter") + } + + // Fallback to env var + if image == "" { + image = defaultFromEnv(image, "RELATED_IMAGE_PGEXPORTER") + } + + return image } // PostgresContainerImageString returns the container image to use for PostgreSQL (from string params). @@ -117,10 +199,20 @@ func PostgresContainerImageString(image string, postgresVersion int, postGISVers // Made as a wrapper of PostgresContainerImageString for compat reasons func PostgresContainerImage(cluster *v1beta1.PostgresCluster) string { image := cluster.Spec.Image - postgresVersion := cluster.Spec.PostgresVersion - postGISVersion := cluster.Spec.PostGISVersion - return PostgresContainerImageString(image, postgresVersion, postGISVersion) + // Try operator-wide config first + if image == "" { + image = getImageFromOperatorConfig(cluster, "postgres") + } + + // Fallback to env var + if image == "" { + postgresVersion := cluster.Spec.PostgresVersion + postGISVersion := cluster.Spec.PostGISVersion + image = PostgresContainerImageString("", postgresVersion, postGISVersion) + } + + return image } // PGONamespace returns the namespace where the PGO is running, diff --git a/percona/config/images/config.go b/percona/config/images/config.go new file mode 100644 index 000000000..6dbbb2565 --- /dev/null +++ b/percona/config/images/config.go @@ -0,0 +1,31 @@ +// Copyright 2021 - 2026 Percona, LLC +// +// SPDX-License-Identifier: Apache-2.0 + +package images + +import ( + _ "embed" + "fmt" + + "gopkg.in/yaml.v3" +) + +//go:embed default-images.yaml +var defaultImagesYAML []byte + +// DefaultConfig holds the default image configuration loaded from embedded YAML +var DefaultConfig *DefaultImagesConfig + +// mustLoadDefault parses the embedded default-images.yaml and returns the configuration +func mustLoadDefault() *DefaultImagesConfig { + var cfg DefaultImagesConfig + if err := yaml.Unmarshal(defaultImagesYAML, &cfg); err != nil { + panic(fmt.Sprintf("failed to parse embedded default images: %v", err)) + } + return &cfg +} + +func init() { + DefaultConfig = mustLoadDefault() +} diff --git a/percona/config/images/default-images.yaml b/percona/config/images/default-images.yaml new file mode 100644 index 000000000..18989715f --- /dev/null +++ b/percona/config/images/default-images.yaml @@ -0,0 +1,98 @@ +# Default image configuration for Percona PostgreSQL Operator +# This file is embedded into the operator binary at build time +# Users can override these values via ConfigMap or environment variable + +# Global registry for all images +registry: docker.io + +# Versions define repositories and tags per CR version +versions: + - crVersion: "3.0.0" + repositories: + postgres: percona/percona-distribution-postgresql + postgresGIS: percona/percona-distribution-postgresql-with-postgis + pgbackrest: percona/percona-pgbackrest + pgbouncer: percona/percona-pgbouncer + tags: + postgres: + "14": "14.23-1" + "15": "15.18-1" + "16": "16.14-1" + "17": "17.10-1" + "18": "18.4-1" + postgresGIS: + "14": "14.23-2" + "15": "15.18-2" + "16": "16.14-2" + "17": "17.10-2" + "18": "18.4-2" + pgbackrest: "2.58.0-2" + pgbouncer: "1.25.2-1" + + - crVersion: "2.9.0" + repositories: + postgres: percona/percona-distribution-postgresql + postgresGIS: percona/percona-distribution-postgresql-with-postgis + pgbackrest: percona/percona-pgbackrest + pgbouncer: percona/percona-pgbouncer + tags: + postgres: + "14": "14.22-1" + "15": "15.17-1" + "16": "16.13-1" + "17": "17.9-1" + "18": "18.3-1" + postgresGIS: + "14": "14.22-1" + "15": "15.17-1" + "16": "16.13-1" + "17": "17.9-1" + "18": "18.3-1" + pgbackrest: "2.58.0-1" + pgbouncer: "1.25.1-1" + + - crVersion: "2.8.1" + repositories: + postgres: percona/percona-distribution-postgresql + postgresGIS: percona/percona-postgresql-operator + pgbackrest: percona/percona-pgbackrest + pgbouncer: percona/percona-pgbouncer + tags: + postgres: + "13": "13.23-1" + "14": "14.20-1" + "15": "15.15-1" + "16": "16.11-1" + "17": "17.7-1" + "18": "18.1-1" + postgresGIS: + "13": "2.8.1-ppg13.23-postgres-gis3.3.8" + "14": "2.8.1-ppg14.20-postgres-gis3.3.8" + "15": "2.8.1-ppg15.15-postgres-gis3.3.8" + "16": "2.8.1-ppg16.11-postgres-gis3.3.8" + "17": "2.8.1-ppg17.7-postgres-gis3.5.4" + "18": "2.8.1-ppg18.1-postgres-gis3.5.4" + pgbackrest: "2.57.0-1" + pgbouncer: "1.25.0-1" + + - crVersion: "2.8.0" + repositories: + postgres: percona/percona-distribution-postgresql + postgresGIS: percona/percona-postgresql-operator + pgbackrest: percona/percona-pgbackrest + pgbouncer: percona/percona-pgbouncer + tags: + postgres: + "13": "13.22-1" + "14": "14.19-1" + "15": "15.14-1" + "16": "16.10-1" + "17": "17.6-1" + postgresGIS: + "13": "2.8.0-ppg13.22-postgres-gis3.3.8" + "14": "2.8.0-ppg14.19-postgres-gis3.3.8" + "15": "2.8.0-ppg15.14-postgres-gis3.3.8" + "16": "2.8.0-ppg16.10-postgres-gis3.3.8" + "17": "2.8.0-ppg17.6-postgres-gis3.3.8" + pgbackrest: "2.56.0-1" + pgbouncer: "1.24.1-1" diff --git a/percona/config/images/loader.go b/percona/config/images/loader.go new file mode 100644 index 000000000..644961537 --- /dev/null +++ b/percona/config/images/loader.go @@ -0,0 +1,169 @@ +// Copyright 2021 - 2026 Percona, LLC +// +// SPDX-License-Identifier: Apache-2.0 + +package images + +import ( + "context" + "fmt" + "os" + + "gopkg.in/yaml.v3" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + // DefaultConfigMapName is the default name of the ConfigMap containing image configuration + DefaultConfigMapName = "percona-pg-image-config" + // DefaultConfigMapKey is the key in the ConfigMap data containing the YAML configuration + DefaultConfigMapKey = "images.yaml" + + // EnvImageConfigPath environment variable for custom config file path + EnvImageConfigPath = "PERCONA_PG_IMAGE_CONFIG" + // EnvImageConfigCM environment variable for custom ConfigMap name + EnvImageConfigCM = "PERCONA_PG_IMAGE_CONFIG_CM" + // EnvImageConfigNS environment variable for custom ConfigMap namespace + EnvImageConfigNS = "PERCONA_PG_IMAGE_CONFIG_NS" +) + +// DefaultConfigMapNamespace returns the default namespace for the image configuration ConfigMap. +// It uses the operator's namespace from PGO_NAMESPACE environment variable. +func DefaultConfigMapNamespace() string { + ns := os.Getenv("PGO_NAMESPACE") + if ns == "" { + // Fallback to default namespace if PGO_NAMESPACE is not set + return "postgres-operator" + } + return ns +} + +// ImageConfigLoader handles loading and merging of image configurations +type ImageConfigLoader struct { + embedded *DefaultImagesConfig + userConfig *DefaultImagesConfig + merged *DefaultImagesConfig +} + +// NewImageConfigLoader creates a new ImageConfigLoader with embedded defaults +func NewImageConfigLoader() *ImageConfigLoader { + return &ImageConfigLoader{ + embedded: DefaultConfig, + } +} + +// Embedded returns the embedded default configuration +func (l *ImageConfigLoader) Embedded() *DefaultImagesConfig { + return l.embedded +} + +// LoadUserConfigFromFile loads user configuration from a YAML file +func (l *ImageConfigLoader) LoadUserConfigFromFile(path string) error { + data, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("read config file: %w", err) + } + + var cfg DefaultImagesConfig + if err := yaml.Unmarshal(data, &cfg); err != nil { + return fmt.Errorf("parse config file: %w", err) + } + + l.userConfig = &cfg + return nil +} + +// LoadUserConfigFromConfigMap loads user configuration from a Kubernetes ConfigMap +func (l *ImageConfigLoader) LoadUserConfigFromConfigMap( + ctx context.Context, + k8sClient client.Client, + cmName, cmNamespace, cmKey string, +) error { + cm := &corev1.ConfigMap{} + err := k8sClient.Get(ctx, types.NamespacedName{ + Name: cmName, Namespace: cmNamespace, + }, cm) + if err != nil { + return fmt.Errorf("get ConfigMap: %w", err) + } + + data, ok := cm.Data[cmKey] + if !ok { + return fmt.Errorf("key %q not found in ConfigMap", cmKey) + } + + var cfg DefaultImagesConfig + if err := yaml.Unmarshal([]byte(data), &cfg); err != nil { + return fmt.Errorf("parse ConfigMap data: %w", err) + } + + l.userConfig = &cfg + return nil +} + +// LoadAuto attempts to load user configuration from environment or ConfigMap +// If neither is available, it continues with embedded defaults +func (l *ImageConfigLoader) LoadAuto(ctx context.Context, k8sClient client.Client) error { + // Check environment variable for file path first + if configPath := os.Getenv(EnvImageConfigPath); configPath != "" { + return l.LoadUserConfigFromFile(configPath) + } + + // Determine ConfigMap name and namespace + cmName := DefaultConfigMapName + if env := os.Getenv(EnvImageConfigCM); env != "" { + cmName = env + } + + cmNamespace := DefaultConfigMapNamespace() + if env := os.Getenv(EnvImageConfigNS); env != "" { + cmNamespace = env + } + + // Only try ConfigMap if client is available + if k8sClient != nil { + err := l.LoadUserConfigFromConfigMap(ctx, k8sClient, cmName, cmNamespace, DefaultConfigMapKey) + if err != nil { + // Log but don't fail - use defaults + // The caller should handle logging + } + } + + return nil +} + +// Merge combines user configuration with embedded configuration +// User values take precedence over embedded defaults +func (l *ImageConfigLoader) Merge() error { + if l.userConfig == nil { + l.merged = l.embedded + return nil + } + + l.merged = DeepMergeConfigs(l.embedded, l.userConfig) + return nil +} + +// GetMergedConfig returns the merged configuration +func (l *ImageConfigLoader) GetMergedConfig() *DefaultImagesConfig { + return l.merged +} + +// InitializeGlobalConfig initializes the global image configuration +// It attempts to load user configuration and merges it with embedded defaults +func InitializeGlobalConfig(ctx context.Context, k8sClient client.Client) error { + loader := NewImageConfigLoader() + + if err := loader.LoadAuto(ctx, k8sClient); err != nil { + return err + } + + if err := loader.Merge(); err != nil { + return err + } + + SetGlobalConfig(loader.GetMergedConfig()) + return nil +} diff --git a/percona/config/images/merge.go b/percona/config/images/merge.go new file mode 100644 index 000000000..c7ca0dabc --- /dev/null +++ b/percona/config/images/merge.go @@ -0,0 +1,110 @@ +// Copyright 2021 - 2026 Percona, LLC +// +// SPDX-License-Identifier: Apache-2.0 + +package images + +// DeepMergeConfigs merges user configuration into base configuration +// User values take precedence over base values at all levels +func DeepMergeConfigs(base, user *DefaultImagesConfig) *DefaultImagesConfig { + if base == nil { + return user + } + if user == nil { + return base + } + + result := &DefaultImagesConfig{ + Registry: pickString(user.Registry, base.Registry), + Versions: make([]VersionImages, 0), + } + + // Create map for user versions for quick lookup + userVersions := make(map[string]*VersionImages) + for i := range user.Versions { + userVersions[user.Versions[i].CRVersion] = &user.Versions[i] + } + + // Process base versions + for _, baseVer := range base.Versions { + mergedVer := VersionImages{ + CRVersion: baseVer.CRVersion, + Repositories: make(map[string]string), + Tags: baseVer.Tags, + } + + // Copy base repositories + for k, v := range baseVer.Repositories { + mergedVer.Repositories[k] = v + } + + // Merge user overrides if present for this CR version + if userVer, ok := userVersions[baseVer.CRVersion]; ok { + // Override repositories + for k, v := range userVer.Repositories { + mergedVer.Repositories[k] = v + } + mergedVer.Tags = mergeTags(baseVer.Tags, userVer.Tags) + delete(userVersions, baseVer.CRVersion) + } + + result.Versions = append(result.Versions, mergedVer) + } + + // Add new user versions not in base + for crVer, userVer := range userVersions { + newVer := VersionImages{ + CRVersion: crVer, + Repositories: make(map[string]string), + Tags: userVer.Tags, + } + for k, v := range userVer.Repositories { + newVer.Repositories[k] = v + } + result.Versions = append(result.Versions, newVer) + } + + return result +} + +// mergeTags merges user tags into base tags +func mergeTags(base, user VersionTags) VersionTags { + result := base + + // Merge postgres tags map + if len(user.Postgres) > 0 { + if result.Postgres == nil { + result.Postgres = make(map[string]string) + } + for k, v := range user.Postgres { + result.Postgres[k] = v + } + } + + // Merge postgresGIS tags map + if len(user.PostgresGIS) > 0 { + if result.PostgresGIS == nil { + result.PostgresGIS = make(map[string]string) + } + for k, v := range user.PostgresGIS { + result.PostgresGIS[k] = v + } + } + + // Override single-value tags if non-empty + result.PGBackRest = pickString(user.PGBackRest, base.PGBackRest) + result.PGBouncer = pickString(user.PGBouncer, base.PGBouncer) + result.PGAdmin = pickString(user.PGAdmin, base.PGAdmin) + result.PGExporter = pickString(user.PGExporter, base.PGExporter) + result.StandalonePGAdmin = pickString(user.StandalonePGAdmin, base.StandalonePGAdmin) + + return result +} + +// pickString returns user value if non-empty, otherwise base value +func pickString(user, base string) string { + if user != "" { + return user + } + return base +} diff --git a/percona/config/images/merge_test.go b/percona/config/images/merge_test.go new file mode 100644 index 000000000..a5ab1f368 --- /dev/null +++ b/percona/config/images/merge_test.go @@ -0,0 +1,254 @@ +// Copyright 2021 - 2026 Percona, LLC +// +// SPDX-License-Identifier: Apache-2.0 + +package images + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestDeepMergeConfigs(t *testing.T) { + t.Run("user is nil", func(t *testing.T) { + base := &DefaultImagesConfig{Registry: "docker.io"} + result := DeepMergeConfigs(base, nil) + assert.Equal(t, base, result) + }) + + t.Run("base is nil", func(t *testing.T) { + user := &DefaultImagesConfig{Registry: "quay.io"} + result := DeepMergeConfigs(nil, user) + assert.Equal(t, user, result) + }) + + t.Run("both nil", func(t *testing.T) { + result := DeepMergeConfigs(nil, nil) + assert.Nil(t, result) + }) + + t.Run("registry override", func(t *testing.T) { + base := &DefaultImagesConfig{Registry: "docker.io"} + user := &DefaultImagesConfig{Registry: "quay.io"} + result := DeepMergeConfigs(base, user) + assert.Equal(t, "quay.io", result.Registry) + }) + + t.Run("registry from base when user empty", func(t *testing.T) { + base := &DefaultImagesConfig{Registry: "docker.io"} + user := &DefaultImagesConfig{Registry: ""} + result := DeepMergeConfigs(base, user) + assert.Equal(t, "docker.io", result.Registry) + }) + + t.Run("merge versions - override existing", func(t *testing.T) { + base := &DefaultImagesConfig{ + Registry: "docker.io", + Versions: []VersionImages{ + { + CRVersion: "2.9.0", + Repositories: map[string]string{ + "postgres": "percona/postgres", + "pgbackrest": "percona/pgbackrest", + }, + Tags: VersionTags{ + Postgres: map[string]string{"14": "1.0", "15": "2.0"}, + PGBackRest: "2.9.0", + }, + }, + }, + } + + user := &DefaultImagesConfig{ + Registry: "", + Versions: []VersionImages{ + { + CRVersion: "2.9.0", + Repositories: map[string]string{ + "postgres": "custom/postgres", + }, + Tags: VersionTags{ + Postgres: map[string]string{"14": "1.1"}, + PGBackRest: "2.9.1", + }, + }, + }, + } + + result := DeepMergeConfigs(base, user) + assert.Equal(t, 1, len(result.Versions)) + assert.Equal(t, "2.9.0", result.Versions[0].CRVersion) + assert.Equal(t, "custom/postgres", result.Versions[0].Repositories["postgres"]) + assert.Equal(t, "percona/pgbackrest", result.Versions[0].Repositories["pgbackrest"]) // from base + assert.Equal(t, "1.1", result.Versions[0].Tags.Postgres["14"]) + assert.Equal(t, "2.0", result.Versions[0].Tags.Postgres["15"]) // from base + assert.Equal(t, "2.9.1", result.Versions[0].Tags.PGBackRest) + }) + + t.Run("merge versions - add new version", func(t *testing.T) { + base := &DefaultImagesConfig{ + Registry: "docker.io", + Versions: []VersionImages{ + { + CRVersion: "2.9.0", + Repositories: map[string]string{ + "postgres": "percona/postgres", + }, + Tags: VersionTags{ + Postgres: map[string]string{"14": "1.0"}, + }, + }, + }, + } + + user := &DefaultImagesConfig{ + Registry: "", + Versions: []VersionImages{ + { + CRVersion: "2.10.0", + Repositories: map[string]string{ + "postgres": "percona/postgres", + }, + Tags: VersionTags{ + Postgres: map[string]string{"14": "2.0"}, + }, + }, + }, + } + + result := DeepMergeConfigs(base, user) + assert.Equal(t, 2, len(result.Versions)) + // Check that both versions exist + has29 := false + has210 := false + for _, v := range result.Versions { + if v.CRVersion == "2.9.0" { + has29 = true + } + if v.CRVersion == "2.10.0" { + has210 = true + } + } + assert.True(t, has29) + assert.True(t, has210) + }) + + t.Run("merge postgres tags map", func(t *testing.T) { + base := &DefaultImagesConfig{ + Versions: []VersionImages{ + { + CRVersion: "2.9.0", + Repositories: map[string]string{ + "postgres": "percona/postgres", + }, + Tags: VersionTags{ + Postgres: map[string]string{"14": "1.0", "15": "2.0"}, + }, + }, + }, + } + + user := &DefaultImagesConfig{ + Versions: []VersionImages{ + { + CRVersion: "2.9.0", + Repositories: map[string]string{ + "postgres": "percona/postgres", + }, + Tags: VersionTags{ + Postgres: map[string]string{"16": "3.0"}, + }, + }, + }, + } + + result := DeepMergeConfigs(base, user) + assert.Equal(t, 3, len(result.Versions[0].Tags.Postgres)) + assert.Equal(t, "1.0", result.Versions[0].Tags.Postgres["14"]) + assert.Equal(t, "2.0", result.Versions[0].Tags.Postgres["15"]) + assert.Equal(t, "3.0", result.Versions[0].Tags.Postgres["16"]) + }) + + t.Run("merge postgresGIS tags map", func(t *testing.T) { + base := &DefaultImagesConfig{ + Versions: []VersionImages{ + { + CRVersion: "2.9.0", + Repositories: map[string]string{ + "postgresGIS": "percona/postgis", + }, + Tags: VersionTags{ + PostgresGIS: map[string]string{"14-gis-3.3": "1.0"}, + }, + }, + }, + } + + user := &DefaultImagesConfig{ + Versions: []VersionImages{ + { + CRVersion: "2.9.0", + Repositories: map[string]string{ + "postgresGIS": "percona/postgis", + }, + Tags: VersionTags{ + PostgresGIS: map[string]string{"15-gis-3.3": "2.0"}, + }, + }, + }, + } + + result := DeepMergeConfigs(base, user) + assert.Equal(t, 2, len(result.Versions[0].Tags.PostgresGIS)) + assert.Equal(t, "1.0", result.Versions[0].Tags.PostgresGIS["14-gis-3.3"]) + assert.Equal(t, "2.0", result.Versions[0].Tags.PostgresGIS["15-gis-3.3"]) + }) +} + +func TestMergeTags(t *testing.T) { + t.Run("merge single value tags", func(t *testing.T) { + base := VersionTags{ + PGBackRest: "2.9.0", + PGBouncer: "2.9.0", + PGAdmin: "2.9.0", + PGExporter: "2.9.0", + } + + user := VersionTags{ + PGBackRest: "2.10.0", + PGAdmin: "2.10.0", + } + + result := mergeTags(base, user) + assert.Equal(t, "2.10.0", result.PGBackRest) + assert.Equal(t, "2.9.0", result.PGBouncer) // from base + assert.Equal(t, "2.10.0", result.PGAdmin) + assert.Equal(t, "2.9.0", result.PGExporter) // from base + }) + + t.Run("empty user values don't override", func(t *testing.T) { + base := VersionTags{ + PGBackRest: "2.9.0", + } + + user := VersionTags{ + PGBackRest: "", + } + + result := mergeTags(base, user) + assert.Equal(t, "2.9.0", result.PGBackRest) + }) +} + +func TestPickString(t *testing.T) { + t.Run("user non-empty", func(t *testing.T) { + result := pickString("user", "base") + assert.Equal(t, "user", result) + }) + + t.Run("user empty", func(t *testing.T) { + result := pickString("", "base") + assert.Equal(t, "base", result) + }) +} diff --git a/percona/config/images/types.go b/percona/config/images/types.go new file mode 100644 index 000000000..c4156a3da --- /dev/null +++ b/percona/config/images/types.go @@ -0,0 +1,120 @@ +// Copyright 2021 - 2026 Percona, LLC +// +// SPDX-License-Identifier: Apache-2.0 + +package images + +// VersionImages defines image configuration for a specific CR version +type VersionImages struct { + CRVersion string `json:"crVersion"` + Repositories map[string]string `json:"repositories"` + Tags VersionTags `json:"tags"` +} + +// VersionTags separates PostgreSQL tags (per version) from component tags (single) +type VersionTags struct { + Postgres map[string]string `json:"postgres"` + PostgresGIS map[string]string `json:"postgresGIS"` + PGBackRest string `json:"pgbackrest"` + PGBouncer string `json:"pgbouncer"` + PGAdmin string `json:"pgadmin"` + PGExporter string `json:"pgexporter"` + StandalonePGAdmin string `json:"standalonePGAdmin"` +} + +// DefaultImagesConfig is the root configuration structure +type DefaultImagesConfig struct { + // Global registry for all images (e.g., "docker.io", "quay.io") + Registry string `json:"registry"` + Versions []VersionImages `json:"versions"` +} + +// GetImage builds full image reference: registry/repository:tag +func (c *DefaultImagesConfig) GetImage(crVersion, component, pgVersion string) string { + versionConfig := c.getVersionConfig(crVersion) + if versionConfig == nil { + return "" + } + + repo := versionConfig.Repositories[component] + if repo == "" { + return "" + } + + tag := c.getTag(versionConfig, component, pgVersion) + if tag == "" { + return "" + } + + return c.buildImageRef(repo, tag) +} + +// getVersionConfig returns the VersionImages for a specific CR version +func (c *DefaultImagesConfig) getVersionConfig(crVersion string) *VersionImages { + for i := range c.Versions { + if c.Versions[i].CRVersion == crVersion { + return &c.Versions[i] + } + } + return nil +} + +// getTag returns the tag for a specific component and PostgreSQL version +func (c *DefaultImagesConfig) getTag(v *VersionImages, component, pgVersion string) string { + switch component { + case "postgres": + if v.Tags.Postgres != nil { + return v.Tags.Postgres[pgVersion] + } + case "postgresGIS": + if v.Tags.PostgresGIS != nil { + return v.Tags.PostgresGIS[pgVersion] + } + case "pgbackrest": + return v.Tags.PGBackRest + case "pgbouncer": + return v.Tags.PGBouncer + case "pgadmin": + return v.Tags.PGAdmin + case "pgexporter": + return v.Tags.PGExporter + case "standalonePGAdmin": + return v.Tags.StandalonePGAdmin + } + return "" +} + +// buildImageRef constructs the full image reference from repository and tag +func (c *DefaultImagesConfig) buildImageRef(repository, tag string) string { + if repository == "" || tag == "" { + return "" + } + + image := repository + if c.Registry != "" { + image = c.Registry + "/" + repository + } + + return image + ":" + tag +} + +// Global config accessor +var globalConfig *DefaultImagesConfig + +// SetGlobalConfig sets the global image configuration +func SetGlobalConfig(config *DefaultImagesConfig) { + globalConfig = config +} + +// GetGlobalConfig returns the global image configuration +func GetGlobalConfig() *DefaultImagesConfig { + return globalConfig +} + +// GetImageForCluster is a convenience function to get image for a specific CR version and component +func GetImageForCluster(crVersion, component, pgVersion string) string { + if globalConfig == nil { + return "" + } + return globalConfig.GetImage(crVersion, component, pgVersion) +} diff --git a/percona/config/images/types_test.go b/percona/config/images/types_test.go new file mode 100644 index 000000000..9eb03f494 --- /dev/null +++ b/percona/config/images/types_test.go @@ -0,0 +1,203 @@ +// Copyright 2021 - 2026 Percona, LLC +// +// SPDX-License-Identifier: Apache-2.0 + +package images + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestDefaultImagesConfig_GetImage(t *testing.T) { + cfg := &DefaultImagesConfig{ + Registry: "docker.io", + Versions: []VersionImages{ + { + CRVersion: "2.9.0", + Repositories: map[string]string{ + "postgres": "percona/percona-postgresql", + "pgbackrest": "percona/pgbackrest", + "pgbouncer": "percona/pgbouncer", + "pgadmin": "percona/pgadmin4", + "pgexporter": "percona/pg_exporter", + "postgresGIS": "percona/percona-postgresql-gis", + }, + Tags: VersionTags{ + Postgres: map[string]string{ + "14": "14.10-1", + "15": "15.5-1", + }, + PostgresGIS: map[string]string{ + "14": "14.10-3.3-1", + }, + PGBackRest: "2.9.0", + PGBouncer: "2.9.0", + PGAdmin: "2.9.0", + PGExporter: "2.9.0", + }, + }, + }, + } + + tests := []struct { + name string + crVersion string + component string + pgVersion string + expected string + }{ + { + name: "PostgreSQL 14", + crVersion: "2.9.0", + component: "postgres", + pgVersion: "14", + expected: "docker.io/percona/percona-postgresql:14.10-1", + }, + { + name: "PostgreSQL 15", + crVersion: "2.9.0", + component: "postgres", + pgVersion: "15", + expected: "docker.io/percona/percona-postgresql:15.5-1", + }, + { + name: "PostGIS 14", + crVersion: "2.9.0", + component: "postgresGIS", + pgVersion: "14", + expected: "docker.io/percona/percona-postgresql-gis:14.10-3.3-1", + }, + { + name: "pgBackRest", + crVersion: "2.9.0", + component: "pgbackrest", + pgVersion: "", + expected: "docker.io/percona/pgbackrest:2.9.0", + }, + { + name: "pgBouncer", + crVersion: "2.9.0", + component: "pgbouncer", + pgVersion: "", + expected: "docker.io/percona/pgbouncer:2.9.0", + }, + { + name: "pgAdmin", + crVersion: "2.9.0", + component: "pgadmin", + pgVersion: "", + expected: "docker.io/percona/pgadmin4:2.9.0", + }, + { + name: "pgExporter", + crVersion: "2.9.0", + component: "pgexporter", + pgVersion: "", + expected: "docker.io/percona/pg_exporter:2.9.0", + }, + { + name: "unknown CR version", + crVersion: "9.9.9", + component: "postgres", + pgVersion: "14", + expected: "", + }, + { + name: "unknown PostgreSQL version", + crVersion: "2.9.0", + component: "postgres", + pgVersion: "99", + expected: "", + }, + { + name: "unknown component", + crVersion: "2.9.0", + component: "unknown", + pgVersion: "14", + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := cfg.GetImage(tt.crVersion, tt.component, tt.pgVersion) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestDefaultImagesConfig_GetVersionConfig(t *testing.T) { + cfg := &DefaultImagesConfig{ + Registry: "docker.io", + Versions: []VersionImages{ + { + CRVersion: "2.9.0", + Repositories: map[string]string{ + "postgres": "percona/postgres", + }, + Tags: VersionTags{ + Postgres: map[string]string{"14": "1.0"}, + }, + }, + { + CRVersion: "2.10.0", + Repositories: map[string]string{ + "postgres": "percona/postgres", + }, + Tags: VersionTags{ + Postgres: map[string]string{"14": "2.0"}, + }, + }, + }, + } + + t.Run("found", func(t *testing.T) { + result := cfg.getVersionConfig("2.9.0") + assert.NotNil(t, result) + assert.Equal(t, "2.9.0", result.CRVersion) + }) + + t.Run("not found", func(t *testing.T) { + result := cfg.getVersionConfig("9.9.9") + assert.Nil(t, result) + }) +} + +func TestGlobalConfig(t *testing.T) { + t.Run("Set and Get", func(t *testing.T) { + cfg := &DefaultImagesConfig{ + Registry: "test.io", + } + SetGlobalConfig(cfg) + result := GetGlobalConfig() + assert.Equal(t, cfg, result) + }) + + t.Run("GetImageForCluster", func(t *testing.T) { + cfg := &DefaultImagesConfig{ + Registry: "docker.io", + Versions: []VersionImages{ + { + CRVersion: "2.9.0", + Repositories: map[string]string{ + "postgres": "percona/postgres", + }, + Tags: VersionTags{ + Postgres: map[string]string{"14": "1.0"}, + }, + }, + }, + } + SetGlobalConfig(cfg) + result := GetImageForCluster("2.9.0", "postgres", "14") + assert.Equal(t, "docker.io/percona/postgres:1.0", result) + }) + + t.Run("GetImageForCluster with nil config", func(t *testing.T) { + SetGlobalConfig(nil) + result := GetImageForCluster("2.9.0", "postgres", "14") + assert.Equal(t, "", result) + }) +} From 94ed70b06b71eca4980b9ca3ac5705f723c90283 Mon Sep 17 00:00:00 2001 From: jjaruszewski Date: Mon, 15 Jun 2026 14:35:51 +0200 Subject: [PATCH 2/4] Cleanup code, remove standalone pgadmin Signed-off-by: jjaruszewski --- percona/config/images/loader.go | 29 ++++++++++++++---------- percona/config/images/merge.go | 20 ++++++++++++++--- percona/config/images/merge_test.go | 34 ++++++++++++++++++++++++----- percona/config/images/types.go | 16 +++++--------- percona/config/images/types_test.go | 26 ++++++++++++---------- 5 files changed, 84 insertions(+), 41 deletions(-) diff --git a/percona/config/images/loader.go b/percona/config/images/loader.go index 644961537..e4a99342d 100644 --- a/percona/config/images/loader.go +++ b/percona/config/images/loader.go @@ -11,6 +11,7 @@ import ( "gopkg.in/yaml.v3" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -60,15 +61,16 @@ func (l *ImageConfigLoader) Embedded() *DefaultImagesConfig { } // LoadUserConfigFromFile loads user configuration from a YAML file +// Returns error if file doesn't exist or fails to parse func (l *ImageConfigLoader) LoadUserConfigFromFile(path string) error { data, err := os.ReadFile(path) if err != nil { - return fmt.Errorf("read config file: %w", err) + return fmt.Errorf("read config file %q: %w", path, err) } var cfg DefaultImagesConfig if err := yaml.Unmarshal(data, &cfg); err != nil { - return fmt.Errorf("parse config file: %w", err) + return fmt.Errorf("parse config file %q: %w", path, err) } l.userConfig = &cfg @@ -76,6 +78,7 @@ func (l *ImageConfigLoader) LoadUserConfigFromFile(path string) error { } // LoadUserConfigFromConfigMap loads user configuration from a Kubernetes ConfigMap +// Returns error if ConfigMap exists but key is missing or YAML parsing fails func (l *ImageConfigLoader) LoadUserConfigFromConfigMap( ctx context.Context, k8sClient client.Client, @@ -86,17 +89,17 @@ func (l *ImageConfigLoader) LoadUserConfigFromConfigMap( Name: cmName, Namespace: cmNamespace, }, cm) if err != nil { - return fmt.Errorf("get ConfigMap: %w", err) + return fmt.Errorf("get ConfigMap %s/%s: %w", cmNamespace, cmName, err) } data, ok := cm.Data[cmKey] if !ok { - return fmt.Errorf("key %q not found in ConfigMap", cmKey) + return fmt.Errorf("ConfigMap %s/%s exists but key %q is missing", cmNamespace, cmName, cmKey) } var cfg DefaultImagesConfig if err := yaml.Unmarshal([]byte(data), &cfg); err != nil { - return fmt.Errorf("parse ConfigMap data: %w", err) + return fmt.Errorf("ConfigMap %s/%s: failed to parse YAML in key %q: %w", cmNamespace, cmName, cmKey, err) } l.userConfig = &cfg @@ -122,13 +125,17 @@ func (l *ImageConfigLoader) LoadAuto(ctx context.Context, k8sClient client.Clien cmNamespace = env } - // Only try ConfigMap if client is available - if k8sClient != nil { - err := l.LoadUserConfigFromConfigMap(ctx, k8sClient, cmName, cmNamespace, DefaultConfigMapKey) - if err != nil { - // Log but don't fail - use defaults - // The caller should handle logging + err := l.LoadUserConfigFromConfigMap(ctx, k8sClient, cmName, cmNamespace, DefaultConfigMapKey) + if err != nil { + // Only silently ignore "ConfigMap not found" - this is expected when using defaults + // All other errors (parse failures, permission errors, missing key) should be returned + // because they indicate a misconfigured ConfigMap that the user explicitly created + if errors.IsNotFound(err) { + // Log for visibility that we're using defaults + fmt.Printf("INFO: ConfigMap %s/%s not found, using default image configuration\n", cmNamespace, cmName) + return nil } + return err } return nil diff --git a/percona/config/images/merge.go b/percona/config/images/merge.go index c7ca0dabc..2f8f24309 100644 --- a/percona/config/images/merge.go +++ b/percona/config/images/merge.go @@ -4,6 +4,20 @@ package images +import "maps" + +// DeepCopy returns a deep copy of the VersionTags struct +// This is used to avoid mutating the original when merging configurations +func (v VersionTags) DeepCopy() VersionTags { + return VersionTags{ + PGBackRest: v.PGBackRest, + PGBouncer: v.PGBouncer, + PGAdmin: v.PGAdmin, + Postgres: maps.Clone(v.Postgres), + PostgresGIS: maps.Clone(v.PostgresGIS), + } +} + // DeepMergeConfigs merges user configuration into base configuration // User values take precedence over base values at all levels func DeepMergeConfigs(base, user *DefaultImagesConfig) *DefaultImagesConfig { @@ -68,8 +82,10 @@ func DeepMergeConfigs(base, user *DefaultImagesConfig) *DefaultImagesConfig { } // mergeTags merges user tags into base tags +// Returns a new VersionTags struct without mutating base or user func mergeTags(base, user VersionTags) VersionTags { - result := base + // Create a deep copy to avoid mutating base + result := base.DeepCopy() // Merge postgres tags map if len(user.Postgres) > 0 { @@ -95,8 +111,6 @@ func mergeTags(base, user VersionTags) VersionTags { result.PGBackRest = pickString(user.PGBackRest, base.PGBackRest) result.PGBouncer = pickString(user.PGBouncer, base.PGBouncer) result.PGAdmin = pickString(user.PGAdmin, base.PGAdmin) - result.PGExporter = pickString(user.PGExporter, base.PGExporter) - result.StandalonePGAdmin = pickString(user.StandalonePGAdmin, base.StandalonePGAdmin) return result } diff --git a/percona/config/images/merge_test.go b/percona/config/images/merge_test.go index a5ab1f368..368c3eece 100644 --- a/percona/config/images/merge_test.go +++ b/percona/config/images/merge_test.go @@ -209,10 +209,9 @@ func TestDeepMergeConfigs(t *testing.T) { func TestMergeTags(t *testing.T) { t.Run("merge single value tags", func(t *testing.T) { base := VersionTags{ - PGBackRest: "2.9.0", - PGBouncer: "2.9.0", - PGAdmin: "2.9.0", - PGExporter: "2.9.0", + PGBackRest: "2.9.0", + PGBouncer: "2.9.0", + PGAdmin: "2.9.0", } user := VersionTags{ @@ -224,7 +223,6 @@ func TestMergeTags(t *testing.T) { assert.Equal(t, "2.10.0", result.PGBackRest) assert.Equal(t, "2.9.0", result.PGBouncer) // from base assert.Equal(t, "2.10.0", result.PGAdmin) - assert.Equal(t, "2.9.0", result.PGExporter) // from base }) t.Run("empty user values don't override", func(t *testing.T) { @@ -239,6 +237,32 @@ func TestMergeTags(t *testing.T) { result := mergeTags(base, user) assert.Equal(t, "2.9.0", result.PGBackRest) }) + + t.Run("does not mutate base maps", func(t *testing.T) { + base := VersionTags{ + Postgres: map[string]string{"14": "1.0", "15": "2.0"}, + } + + user := VersionTags{ + Postgres: map[string]string{"16": "3.0"}, + } + + // Capture original base map state + originalBasePostgres := make(map[string]string) + for k, v := range base.Postgres { + originalBasePostgres[k] = v + } + + result := mergeTags(base, user) + + // Verify result has merged data + assert.Equal(t, 3, len(result.Postgres)) + assert.Equal(t, "3.0", result.Postgres["16"]) + + // Verify base was NOT mutated + assert.Equal(t, 2, len(base.Postgres), "base map should not have new keys added") + assert.Equal(t, originalBasePostgres, base.Postgres, "base map should be unchanged") + }) } func TestPickString(t *testing.T) { diff --git a/percona/config/images/types.go b/percona/config/images/types.go index c4156a3da..65a75d1b5 100644 --- a/percona/config/images/types.go +++ b/percona/config/images/types.go @@ -13,13 +13,11 @@ type VersionImages struct { // VersionTags separates PostgreSQL tags (per version) from component tags (single) type VersionTags struct { - Postgres map[string]string `json:"postgres"` - PostgresGIS map[string]string `json:"postgresGIS"` - PGBackRest string `json:"pgbackrest"` - PGBouncer string `json:"pgbouncer"` - PGAdmin string `json:"pgadmin"` - PGExporter string `json:"pgexporter"` - StandalonePGAdmin string `json:"standalonePGAdmin"` + Postgres map[string]string `json:"postgres"` + PostgresGIS map[string]string `json:"postgresGIS"` + PGBackRest string `json:"pgbackrest"` + PGBouncer string `json:"pgbouncer"` + PGAdmin string `json:"pgadmin"` } // DefaultImagesConfig is the root configuration structure @@ -76,10 +74,6 @@ func (c *DefaultImagesConfig) getTag(v *VersionImages, component, pgVersion stri return v.Tags.PGBouncer case "pgadmin": return v.Tags.PGAdmin - case "pgexporter": - return v.Tags.PGExporter - case "standalonePGAdmin": - return v.Tags.StandalonePGAdmin } return "" } diff --git a/percona/config/images/types_test.go b/percona/config/images/types_test.go index 9eb03f494..7ba766d94 100644 --- a/percona/config/images/types_test.go +++ b/percona/config/images/types_test.go @@ -32,10 +32,9 @@ func TestDefaultImagesConfig_GetImage(t *testing.T) { PostgresGIS: map[string]string{ "14": "14.10-3.3-1", }, - PGBackRest: "2.9.0", - PGBouncer: "2.9.0", - PGAdmin: "2.9.0", - PGExporter: "2.9.0", + PGBackRest: "2.9.0", + PGBouncer: "2.9.0", + PGAdmin: "2.9.0", }, }, }, @@ -90,13 +89,6 @@ func TestDefaultImagesConfig_GetImage(t *testing.T) { pgVersion: "", expected: "docker.io/percona/pgadmin4:2.9.0", }, - { - name: "pgExporter", - crVersion: "2.9.0", - component: "pgexporter", - pgVersion: "", - expected: "docker.io/percona/pg_exporter:2.9.0", - }, { name: "unknown CR version", crVersion: "9.9.9", @@ -167,6 +159,10 @@ func TestDefaultImagesConfig_GetVersionConfig(t *testing.T) { func TestGlobalConfig(t *testing.T) { t.Run("Set and Get", func(t *testing.T) { + // Capture and restore previous global config + prevConfig := GetGlobalConfig() + t.Cleanup(func() { SetGlobalConfig(prevConfig) }) + cfg := &DefaultImagesConfig{ Registry: "test.io", } @@ -176,6 +172,10 @@ func TestGlobalConfig(t *testing.T) { }) t.Run("GetImageForCluster", func(t *testing.T) { + // Capture and restore previous global config + prevConfig := GetGlobalConfig() + t.Cleanup(func() { SetGlobalConfig(prevConfig) }) + cfg := &DefaultImagesConfig{ Registry: "docker.io", Versions: []VersionImages{ @@ -196,6 +196,10 @@ func TestGlobalConfig(t *testing.T) { }) t.Run("GetImageForCluster with nil config", func(t *testing.T) { + // Capture and restore previous global config + prevConfig := GetGlobalConfig() + t.Cleanup(func() { SetGlobalConfig(prevConfig) }) + SetGlobalConfig(nil) result := GetImageForCluster("2.9.0", "postgres", "14") assert.Equal(t, "", result) From f1fd95df9946d5bb43a59dd6a8c6f5a391dd2385 Mon Sep 17 00:00:00 2001 From: jjaruszewski Date: Mon, 15 Jun 2026 15:24:18 +0200 Subject: [PATCH 3/4] Fix linting issues Signed-off-by: jjaruszewski --- internal/config/config.go | 6 ----- percona/config/images/config.go | 6 ++--- percona/config/images/loader.go | 37 ++++++++++++++++++++--------- percona/config/images/merge_test.go | 12 +++++----- percona/config/images/types.go | 20 ++++++++-------- percona/config/images/types_test.go | 23 +++++++++--------- 6 files changed, 56 insertions(+), 48 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 91ada7954..b85307d73 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -125,12 +125,6 @@ func StandalonePGAdminContainerImage(pgadmin *v1beta1.PGAdmin) string { image = *pgadmin.Spec.Image } - // Try operator-wide config first - if image == "" { - // For standalone pgadmin, we don't have cluster context - // Use a default version or skip - } - // Fallback to env var if image == "" { image = defaultFromEnv(image, "RELATED_IMAGE_STANDALONE_PGADMIN") diff --git a/percona/config/images/config.go b/percona/config/images/config.go index 6dbbb2565..b5e74f7f8 100644 --- a/percona/config/images/config.go +++ b/percona/config/images/config.go @@ -6,9 +6,9 @@ package images import ( _ "embed" - "fmt" - "gopkg.in/yaml.v3" + "github.com/pkg/errors" + "sigs.k8s.io/yaml" ) //go:embed default-images.yaml @@ -21,7 +21,7 @@ var DefaultConfig *DefaultImagesConfig func mustLoadDefault() *DefaultImagesConfig { var cfg DefaultImagesConfig if err := yaml.Unmarshal(defaultImagesYAML, &cfg); err != nil { - panic(fmt.Sprintf("failed to parse embedded default images: %v", err)) + panic(errors.Wrap(err, "failed to parse embedded default images")) } return &cfg } diff --git a/percona/config/images/loader.go b/percona/config/images/loader.go index e4a99342d..85722f484 100644 --- a/percona/config/images/loader.go +++ b/percona/config/images/loader.go @@ -8,12 +8,14 @@ import ( "context" "fmt" "os" + "path/filepath" - "gopkg.in/yaml.v3" + "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/yaml" ) const ( @@ -60,17 +62,30 @@ func (l *ImageConfigLoader) Embedded() *DefaultImagesConfig { return l.embedded } -// LoadUserConfigFromFile loads user configuration from a YAML file -// Returns error if file doesn't exist or fails to parse +// LoadUserConfigFromFile loads user configuration from a YAML file. +// Returns error if file doesn't exist, fails to parse, or path validation fails. func (l *ImageConfigLoader) LoadUserConfigFromFile(path string) error { - data, err := os.ReadFile(path) + // Clean the path first to resolve any .. or . components + cleanPath := filepath.Clean(path) + + // Verify the cleaned path is absolute + if !filepath.IsAbs(cleanPath) { + return errors.Errorf("config file path must be absolute, got: %q", cleanPath) + } + + // Verify file extension is .yaml or .yml + if ext := filepath.Ext(cleanPath); ext != ".yaml" && ext != ".yml" { + return errors.Errorf("config file must have .yaml or .yml extension, got: %q", ext) + } + + data, err := os.ReadFile(cleanPath) if err != nil { - return fmt.Errorf("read config file %q: %w", path, err) + return errors.Wrapf(err, "read config file %q", cleanPath) } var cfg DefaultImagesConfig if err := yaml.Unmarshal(data, &cfg); err != nil { - return fmt.Errorf("parse config file %q: %w", path, err) + return errors.Wrapf(err, "parse config file %q", cleanPath) } l.userConfig = &cfg @@ -89,17 +104,17 @@ func (l *ImageConfigLoader) LoadUserConfigFromConfigMap( Name: cmName, Namespace: cmNamespace, }, cm) if err != nil { - return fmt.Errorf("get ConfigMap %s/%s: %w", cmNamespace, cmName, err) + return errors.Wrapf(err, "get ConfigMap %s/%s", cmNamespace, cmName) } data, ok := cm.Data[cmKey] if !ok { - return fmt.Errorf("ConfigMap %s/%s exists but key %q is missing", cmNamespace, cmName, cmKey) + return errors.Errorf("ConfigMap %s/%s exists but key %q is missing", cmNamespace, cmName, cmKey) } var cfg DefaultImagesConfig if err := yaml.Unmarshal([]byte(data), &cfg); err != nil { - return fmt.Errorf("ConfigMap %s/%s: failed to parse YAML in key %q: %w", cmNamespace, cmName, cmKey, err) + return errors.Wrapf(err, "ConfigMap %s/%s: failed to parse YAML in key %q", cmNamespace, cmName, cmKey) } l.userConfig = &cfg @@ -130,7 +145,7 @@ func (l *ImageConfigLoader) LoadAuto(ctx context.Context, k8sClient client.Clien // Only silently ignore "ConfigMap not found" - this is expected when using defaults // All other errors (parse failures, permission errors, missing key) should be returned // because they indicate a misconfigured ConfigMap that the user explicitly created - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { // Log for visibility that we're using defaults fmt.Printf("INFO: ConfigMap %s/%s not found, using default image configuration\n", cmNamespace, cmName) return nil diff --git a/percona/config/images/merge_test.go b/percona/config/images/merge_test.go index 368c3eece..bdeeff654 100644 --- a/percona/config/images/merge_test.go +++ b/percona/config/images/merge_test.go @@ -77,7 +77,7 @@ func TestDeepMergeConfigs(t *testing.T) { } result := DeepMergeConfigs(base, user) - assert.Equal(t, 1, len(result.Versions)) + assert.Len(t, result.Versions, 1) assert.Equal(t, "2.9.0", result.Versions[0].CRVersion) assert.Equal(t, "custom/postgres", result.Versions[0].Repositories["postgres"]) assert.Equal(t, "percona/pgbackrest", result.Versions[0].Repositories["pgbackrest"]) // from base @@ -118,7 +118,7 @@ func TestDeepMergeConfigs(t *testing.T) { } result := DeepMergeConfigs(base, user) - assert.Equal(t, 2, len(result.Versions)) + assert.Len(t, result.Versions, 2) // Check that both versions exist has29 := false has210 := false @@ -164,7 +164,7 @@ func TestDeepMergeConfigs(t *testing.T) { } result := DeepMergeConfigs(base, user) - assert.Equal(t, 3, len(result.Versions[0].Tags.Postgres)) + assert.Len(t, result.Versions[0].Tags.Postgres, 3) assert.Equal(t, "1.0", result.Versions[0].Tags.Postgres["14"]) assert.Equal(t, "2.0", result.Versions[0].Tags.Postgres["15"]) assert.Equal(t, "3.0", result.Versions[0].Tags.Postgres["16"]) @@ -200,7 +200,7 @@ func TestDeepMergeConfigs(t *testing.T) { } result := DeepMergeConfigs(base, user) - assert.Equal(t, 2, len(result.Versions[0].Tags.PostgresGIS)) + assert.Len(t, result.Versions[0].Tags.PostgresGIS, 2) assert.Equal(t, "1.0", result.Versions[0].Tags.PostgresGIS["14-gis-3.3"]) assert.Equal(t, "2.0", result.Versions[0].Tags.PostgresGIS["15-gis-3.3"]) }) @@ -256,11 +256,11 @@ func TestMergeTags(t *testing.T) { result := mergeTags(base, user) // Verify result has merged data - assert.Equal(t, 3, len(result.Postgres)) + assert.Len(t, result.Postgres, 3) assert.Equal(t, "3.0", result.Postgres["16"]) // Verify base was NOT mutated - assert.Equal(t, 2, len(base.Postgres), "base map should not have new keys added") + assert.Len(t, base.Postgres, 2, "base map should not have new keys added") assert.Equal(t, originalBasePostgres, base.Postgres, "base map should be unchanged") }) } diff --git a/percona/config/images/types.go b/percona/config/images/types.go index 65a75d1b5..9a5378263 100644 --- a/percona/config/images/types.go +++ b/percona/config/images/types.go @@ -6,25 +6,25 @@ package images // VersionImages defines image configuration for a specific CR version type VersionImages struct { - CRVersion string `json:"crVersion"` - Repositories map[string]string `json:"repositories"` - Tags VersionTags `json:"tags"` + CRVersion string `json:"crVersion" yaml:"crVersion"` + Repositories map[string]string `json:"repositories" yaml:"repositories"` + Tags VersionTags `json:"tags" yaml:"tags"` } // VersionTags separates PostgreSQL tags (per version) from component tags (single) type VersionTags struct { - Postgres map[string]string `json:"postgres"` - PostgresGIS map[string]string `json:"postgresGIS"` - PGBackRest string `json:"pgbackrest"` - PGBouncer string `json:"pgbouncer"` - PGAdmin string `json:"pgadmin"` + Postgres map[string]string `json:"postgres" yaml:"postgres"` + PostgresGIS map[string]string `json:"postgresGIS" yaml:"postgresGIS"` + PGBackRest string `json:"pgbackrest" yaml:"pgbackrest"` + PGBouncer string `json:"pgbouncer" yaml:"pgbouncer"` + PGAdmin string `json:"pgadmin" yaml:"pgadmin"` } // DefaultImagesConfig is the root configuration structure type DefaultImagesConfig struct { // Global registry for all images (e.g., "docker.io", "quay.io") - Registry string `json:"registry"` - Versions []VersionImages `json:"versions"` + Registry string `json:"registry" yaml:"registry"` + Versions []VersionImages `json:"versions" yaml:"versions"` } // GetImage builds full image reference: registry/repository:tag diff --git a/percona/config/images/types_test.go b/percona/config/images/types_test.go index 7ba766d94..a7f322ec7 100644 --- a/percona/config/images/types_test.go +++ b/percona/config/images/types_test.go @@ -17,12 +17,11 @@ func TestDefaultImagesConfig_GetImage(t *testing.T) { { CRVersion: "2.9.0", Repositories: map[string]string{ - "postgres": "percona/percona-postgresql", - "pgbackrest": "percona/pgbackrest", - "pgbouncer": "percona/pgbouncer", - "pgadmin": "percona/pgadmin4", - "pgexporter": "percona/pg_exporter", - "postgresGIS": "percona/percona-postgresql-gis", + "postgres": "percona/percona-postgresql", + "pgbackrest": "percona/pgbackrest", + "pgbouncer": "percona/pgbouncer", + "pgadmin": "percona/pgadmin4", + "postgresGIS": "percona/percona-postgresql-gis", }, Tags: VersionTags{ Postgres: map[string]string{ @@ -41,11 +40,11 @@ func TestDefaultImagesConfig_GetImage(t *testing.T) { } tests := []struct { - name string - crVersion string - component string - pgVersion string - expected string + name string + crVersion string + component string + pgVersion string + expected string }{ { name: "PostgreSQL 14", @@ -202,6 +201,6 @@ func TestGlobalConfig(t *testing.T) { SetGlobalConfig(nil) result := GetImageForCluster("2.9.0", "postgres", "14") - assert.Equal(t, "", result) + assert.Empty(t, result) }) } From 86382a2e5e366b7885c1854cb0f50ff5cac9ce61 Mon Sep 17 00:00:00 2001 From: jjaruszewski Date: Mon, 15 Jun 2026 15:57:30 +0200 Subject: [PATCH 4/4] Fix more issues Signed-off-by: jjaruszewski --- internal/config/config.go | 17 ++++----- percona/config/images/loader.go | 12 ++++--- percona/config/images/merge.go | 4 +-- percona/config/images/types.go | 22 +++++++++--- percona/config/images/types_test.go | 56 ++++++++--------------------- 5 files changed, 47 insertions(+), 64 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index b85307d73..004fac2a5 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -32,11 +32,6 @@ func getImageFromOperatorConfig(cluster *v1beta1.PostgresCluster, component stri pgVersion := fmt.Sprintf("%d", cluster.Spec.PostgresVersion) - // If PostGIS is enabled, use the postgresGIS component instead - if cluster.Spec.PostGISVersion != "" { - component = "postgresGIS" - } - return images.GetImageForCluster(crVersion, component, pgVersion) } @@ -164,11 +159,6 @@ func PGExporterContainerImage(cluster *v1beta1.PostgresCluster) string { image = cluster.Spec.Monitoring.PGMonitor.Exporter.Image } - // Try operator-wide config first - if image == "" { - image = getImageFromOperatorConfig(cluster, "pgexporter") - } - // Fallback to env var if image == "" { image = defaultFromEnv(image, "RELATED_IMAGE_PGEXPORTER") @@ -196,7 +186,12 @@ func PostgresContainerImage(cluster *v1beta1.PostgresCluster) string { // Try operator-wide config first if image == "" { - image = getImageFromOperatorConfig(cluster, "postgres") + // Use postgresGIS component if PostGIS is enabled + component := "postgres" + if cluster.Spec.PostGISVersion != "" { + component = "postgresGIS" + } + image = getImageFromOperatorConfig(cluster, component) } // Fallback to env var diff --git a/percona/config/images/loader.go b/percona/config/images/loader.go index 85722f484..33082a526 100644 --- a/percona/config/images/loader.go +++ b/percona/config/images/loader.go @@ -6,7 +6,6 @@ package images import ( "context" - "fmt" "os" "path/filepath" @@ -92,8 +91,9 @@ func (l *ImageConfigLoader) LoadUserConfigFromFile(path string) error { return nil } -// LoadUserConfigFromConfigMap loads user configuration from a Kubernetes ConfigMap -// Returns error if ConfigMap exists but key is missing or YAML parsing fails +// LoadUserConfigFromConfigMap loads user configuration from a Kubernetes ConfigMap. +// Returns error if ConfigMap exists but key is missing or YAML parsing fails. +// Returns the raw Kubernetes API error for NotFound so callers can handle it appropriately. func (l *ImageConfigLoader) LoadUserConfigFromConfigMap( ctx context.Context, k8sClient client.Client, @@ -104,6 +104,10 @@ func (l *ImageConfigLoader) LoadUserConfigFromConfigMap( Name: cmName, Namespace: cmNamespace, }, cm) if err != nil { + // Don't wrap NotFound errors so callers can check with apierrors.IsNotFound() + if apierrors.IsNotFound(err) { + return err + } return errors.Wrapf(err, "get ConfigMap %s/%s", cmNamespace, cmName) } @@ -146,8 +150,6 @@ func (l *ImageConfigLoader) LoadAuto(ctx context.Context, k8sClient client.Clien // All other errors (parse failures, permission errors, missing key) should be returned // because they indicate a misconfigured ConfigMap that the user explicitly created if apierrors.IsNotFound(err) { - // Log for visibility that we're using defaults - fmt.Printf("INFO: ConfigMap %s/%s not found, using default image configuration\n", cmNamespace, cmName) return nil } return err diff --git a/percona/config/images/merge.go b/percona/config/images/merge.go index 2f8f24309..7eb9566cf 100644 --- a/percona/config/images/merge.go +++ b/percona/config/images/merge.go @@ -44,7 +44,7 @@ func DeepMergeConfigs(base, user *DefaultImagesConfig) *DefaultImagesConfig { mergedVer := VersionImages{ CRVersion: baseVer.CRVersion, Repositories: make(map[string]string), - Tags: baseVer.Tags, + Tags: baseVer.Tags.DeepCopy(), } // Copy base repositories @@ -70,7 +70,7 @@ func DeepMergeConfigs(base, user *DefaultImagesConfig) *DefaultImagesConfig { newVer := VersionImages{ CRVersion: crVer, Repositories: make(map[string]string), - Tags: userVer.Tags, + Tags: userVer.Tags.DeepCopy(), } for k, v := range userVer.Repositories { newVer.Repositories[k] = v diff --git a/percona/config/images/types.go b/percona/config/images/types.go index 9a5378263..4274eef9d 100644 --- a/percona/config/images/types.go +++ b/percona/config/images/types.go @@ -4,6 +4,10 @@ package images +import ( + "sync" +) + // VersionImages defines image configuration for a specific CR version type VersionImages struct { CRVersion string `json:"crVersion" yaml:"crVersion"` @@ -93,14 +97,22 @@ func (c *DefaultImagesConfig) buildImageRef(repository, tag string) string { } // Global config accessor -var globalConfig *DefaultImagesConfig - -// SetGlobalConfig sets the global image configuration +var ( + globalConfig *DefaultImagesConfig + globalConfigOnce sync.Once +) + +// SetGlobalConfig sets the global image configuration. +// This function should only be called once during initialization. +// Subsequent calls will be ignored. func SetGlobalConfig(config *DefaultImagesConfig) { - globalConfig = config + globalConfigOnce.Do(func() { + globalConfig = config + }) } -// GetGlobalConfig returns the global image configuration +// GetGlobalConfig returns the global image configuration. +// This is safe for concurrent use after initialization. func GetGlobalConfig() *DefaultImagesConfig { return globalConfig } diff --git a/percona/config/images/types_test.go b/percona/config/images/types_test.go index a7f322ec7..1e198c5d2 100644 --- a/percona/config/images/types_test.go +++ b/percona/config/images/types_test.go @@ -157,50 +157,24 @@ func TestDefaultImagesConfig_GetVersionConfig(t *testing.T) { } func TestGlobalConfig(t *testing.T) { - t.Run("Set and Get", func(t *testing.T) { - // Capture and restore previous global config - prevConfig := GetGlobalConfig() - t.Cleanup(func() { SetGlobalConfig(prevConfig) }) - - cfg := &DefaultImagesConfig{ - Registry: "test.io", + t.Run("SetGlobalConfig is idempotent", func(t *testing.T) { + // This test verifies that SetGlobalConfig can only be called once + // The first call sets the config, subsequent calls are ignored + firstCfg := &DefaultImagesConfig{ + Registry: "first.io", } - SetGlobalConfig(cfg) - result := GetGlobalConfig() - assert.Equal(t, cfg, result) - }) - - t.Run("GetImageForCluster", func(t *testing.T) { - // Capture and restore previous global config - prevConfig := GetGlobalConfig() - t.Cleanup(func() { SetGlobalConfig(prevConfig) }) - - cfg := &DefaultImagesConfig{ - Registry: "docker.io", - Versions: []VersionImages{ - { - CRVersion: "2.9.0", - Repositories: map[string]string{ - "postgres": "percona/postgres", - }, - Tags: VersionTags{ - Postgres: map[string]string{"14": "1.0"}, - }, - }, - }, + secondCfg := &DefaultImagesConfig{ + Registry: "second.io", } - SetGlobalConfig(cfg) - result := GetImageForCluster("2.9.0", "postgres", "14") - assert.Equal(t, "docker.io/percona/postgres:1.0", result) - }) - t.Run("GetImageForCluster with nil config", func(t *testing.T) { - // Capture and restore previous global config - prevConfig := GetGlobalConfig() - t.Cleanup(func() { SetGlobalConfig(prevConfig) }) + // First call should set the config + SetGlobalConfig(firstCfg) + result := GetGlobalConfig() + assert.Equal(t, "first.io", result.Registry) - SetGlobalConfig(nil) - result := GetImageForCluster("2.9.0", "postgres", "14") - assert.Empty(t, result) + // Second call should be ignored (sync.Once behavior) + SetGlobalConfig(secondCfg) + result = GetGlobalConfig() + assert.Equal(t, "first.io", result.Registry, "second SetGlobalConfig should be ignored") }) }