Initial global images config#1640
Draft
Kajot-dev wants to merge 4 commits into
Draft
Conversation
Signed-off-by: jjaruszewski <jjaruszewski@man.poznan.pl>
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an operator-wide, YAML-driven image configuration system that can be overridden at runtime (file/ConfigMap) and used as the primary source for component image resolution.
Changes:
- Introduces image config types, embedded defaults (YAML), a loader, and deep-merge logic for user overrides.
- Initializes a global image config at operator startup and updates image selection helpers to consult it before env vars.
- Adds unit tests for image lookup and config merging.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| percona/config/images/types.go | Defines config types and image resolution helpers + global config accessor. |
| percona/config/images/merge.go | Implements deep-merge logic for embedded vs user image configuration. |
| percona/config/images/loader.go | Loads user config from file/ConfigMap and merges into embedded defaults. |
| percona/config/images/config.go | Embeds and parses default-images.yaml into DefaultConfig. |
| percona/config/images/default-images.yaml | Provides embedded default image registry/repositories/tags per CR version. |
| percona/config/images/types_test.go | Adds tests for image resolution and global config accessors. |
| percona/config/images/merge_test.go | Adds tests for deep-merge behavior of registries, versions, and tag maps. |
| internal/config/config.go | Uses operator-wide image config as the first lookup source for component images. |
| cmd/postgres-operator/main.go | Initializes global image config at startup (with fallback to embedded defaults). |
Signed-off-by: jjaruszewski <jjaruszewski@man.poznan.pl>
Signed-off-by: jjaruszewski <jjaruszewski@man.poznan.pl>
Comment on lines
+67
to
+74
| func (l *ImageConfigLoader) LoadUserConfigFromFile(path string) error { | ||
| // 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) | ||
| } |
Signed-off-by: jjaruszewski <jjaruszewski@man.poznan.pl>
Collaborator
commit: 86382a2 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CHANGE DESCRIPTION
Images that should be used are dependent on version of the operator and
crVersion, but the person deploying the postgresql instance migh to not have access to the operator deployment itselft. This also introduces maintenance burden, because teams need to synchronize with the team that manages the operator.This MR introduces default image per crVersion, so operator can automatically pick certified image related to the crVersion of the postgres instance. Images are embedded at build time, but can be overriden via mounted YAML or configmap by the operator administrators (for example, if they download/mirror from a different registry.
DISCLOSURE: This was AI assisted (of course I have reviewed the AI output and take the responsibility for the code ;) )
NOTE: This is early draft, to discuss if this concept does make sense. There are still multiple things that need to be addressed:
If this concept makes sense (please leave feedback!), I'll make a PR do percona docs and the helm chart
CHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
Config/Logging/Testability