feat: add global database extensions flag to controller#375
Open
mahlunar wants to merge 5 commits into
Open
Conversation
Add --global-extensions flag that applies a consistent set of PostgreSQL extensions across all managed databases. The flag accepts a comma-separated list (e.g., --global-extensions=pgcrypto,uuid-ossp,pg_stat_statements). Global extensions are merged with per-database extensions from PostgreSQLDatabase.spec.extensions[] into a unique set before installation. Database-specific extensions take precedence in the merge order. Implementation: - Add GlobalExtensions field to ControllerConfiguration with flag registration - Add GetGlobalExtensions() method with whitespace trimming and empty filtering - Pass global extensions to PostgreSQLDatabaseReconciler - Add mergeExtensions() method using map-based deduplication - Update startup logs to include global extensions Tests: - Flag parsing: empty, single, multiple, whitespace, trailing commas, empty elements, nil handling - Merge logic: empty scenarios, global only, database only, both, duplicates, nil slices - Integration: verify extensions actually install on PostgreSQL with exact match assertions Also includes go fix improvements (slices.Contains, modern range loops). Closes DMAT-64 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
mahlunar
commented
May 27, 2026
- Return empty slice instead of nil in GetGlobalExtensions for empty input - Fix SuperuserRoleName in extensions test to use iam_creator - Quote extension and schema names to handle identifiers with hyphens (e.g., uuid-ossp) - Remove unused fromApiExtensions function - Fix tautological condition in status update switch statement Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
tmablunar
reviewed
May 27, 2026
tmablunar
reviewed
May 27, 2026
| {ExtensionName: "pgcrypto"}, | ||
| }, | ||
| expectedExtensions: []string{"pgcrypto", "uuid-ossp"}, | ||
| }, |
Contributor
There was a problem hiding this comment.
You could add a test where:
- some or all extensions are already installed
- an extension is installed that is not listed - I guess it should be removed? How do we keep track of that?
Contributor
There was a problem hiding this comment.
Can we see who installed the extension? We could have the assumption that all extensions installed by iam_creator are managed by the controller
Member
Author
There was a problem hiding this comment.
I will add the additional test but not add logic for removing in this PR
| result = append(result, trimmed) | ||
| } | ||
| } | ||
| return result |
Contributor
There was a problem hiding this comment.
Do we want to validate early that there are no clearly invalid extensions like some thing with spaces?
Co-authored-by: Tinus Mørch Abell <tmab@lunar.app>
Member
Author
|
I think it is more dangerous to drop the extensions so let's document the behaviour so it is clear. |
- Rename flag to --global-extensions-to-install to clarify that extensions are only installed, never removed - Add validation for extension names to reject invalid characters and spaces early before attempting installation - Add test case for already-installed extensions to verify reconciliation doesn't fail when extensions exist Addresses feedback from @tmablunar on PR #375 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
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
2 participants
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.
Summary
Add
--global-extensionsflag to postgresql-controller that installs a consistent set of PostgreSQL extensions across all managed databases. Global extensions are merged with per-database extensions into a unique set before installation.Why
Follows the pattern of
--user-roleswhich applies global roles to all users. Allows operators to enforce a baseline set of extensions (e.g.,pgcrypto,uuid-ossp,pg_stat_statements) without modifying every PostgreSQLDatabase CRD.Closes DMAT-64
Note
Medium Risk
Reconciliation now installs additional extensions on all managed databases from operator config, which can change production DB state and fail if an extension is unavailable on the server.
Overview
Adds a
--global-extensionscontroller flag (comma-separated names, parsed viaGetGlobalExtensions()with trimming) so operators can enforce a baseline set of PostgreSQL extensions on every reconciled database, similar to--user-roles.During
PostgreSQLDatabasereconciliation, per-CRDspec.extensionsare merged with the global list throughmergeExtensions()(database entries first, deduplicated) beforeEnsurePostgreSQLDatabaseruns. Startup wiring passes the parsed list fromcmd/main.gointo the reconciler, and config logging includes the raw flag value.Coverage adds unit tests for flag parsing and merge behavior plus integration tests that assert extensions are actually installed on PostgreSQL. Unrelated cleanups use
slices.Containsin the user controller and afor rangeloop style in a test helper.Reviewed by Cursor Bugbot for commit 89ce176. Configure here.