Support configurable Helm storage driver (Secret / ConfigMap / Memory / SQL)#1479
Open
funkypenguin wants to merge 1 commit into
Open
Support configurable Helm storage driver (Secret / ConfigMap / Memory / SQL)#1479funkypenguin wants to merge 1 commit into
funkypenguin wants to merge 1 commit into
Conversation
Adds a `--helm-storage-driver` flag and an `HELM_DRIVER` environment
variable fallback that select the Helm release storage backend, mirroring
the Helm CLI's HELM_DRIVER behaviour. Supported values are
`secret`/`secrets`, `configmap`/`configmaps`, `memory`, and `sql`,
matched case-insensitively. Unset preserves the current behaviour
(Secret), so the change is backwards compatible.
The SQL driver reads its connection string from
`HELM_DRIVER_SQL_CONNECTION_STRING` (matching the Helm CLI). It is
useful when:
- Helm release information exceeds the 1MiB Secret size limit;
- the cumulative Secret count is causing cluster-wide pressure;
- compliance requires storing release data outside the cluster.
A `Memory` driver is accepted for parity with Helm itself, but the
flag help marks it as test/dev only, since the storage is
re-initialised on every reconcile.
Implementation notes:
- The driver name is normalised once at startup (with validation),
so an unsupported value fails the controller process rather than
every HelmRelease reconcile.
- SQL drivers are managed by an SQLDriverPool keyed by storage
namespace, sharing one helmdriver.SQL instance per namespace.
Helm v4 does not expose Close on storage.Driver, so connection
pools are released only when the controller exits; the
per-namespace cache bounds the live pool count to the set of
storage namespaces actually in use rather than once per reconcile.
- At startup, when the SQL driver is selected, the controller probes
the database with a transient database/sql.Open + PingContext +
Close to surface invalid DSNs or unreachable backends before the
manager starts. The probe connection is closed cleanly and does
not contribute to the long-lived pool.
- SQL connect/migration errors can echo the connection string; they
are caught at the WithStorage call site and a generic message is
returned, so DSN material cannot leak into HelmRelease status
conditions.
Closes fluxcd#272.
Supersedes fluxcd#760.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: David Young <davidy@funkypenguin.co.nz>
funkypenguin
force-pushed
the
feat/configurable-storage-driver
branch
from
8f9a6e9 to
f38b931
Compare
Member
|
Nice work! But I think we need a bit of discussion for this feature, e.g. here you are supporting it only at the controller level. I can see value in creating spec fields instead. Would you be able to join the open flux dev meetings to discuss this? https://fluxcd.io/community/ |
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.
Overview
Adds a
--helm-storage-driverflag andHELM_DRIVERenvironment-variable fallback that select the Helm release storage backend, mirroring the Helm CLI'sHELM_DRIVER. Supported values:secret/secrets,configmap/configmaps,memory,sql— matched case-insensitively. Unset preserves current behaviour (Secret), so the change is fully backwards compatible.The SQL driver reads its connection string from
HELM_DRIVER_SQL_CONNECTION_STRING(matching Helm CLI).This PR is a fresh take on the long-stalled #760 (open since 2023). I rebased it against current
main, ported it to the post-Helm-v4 /ConfigFactoryera, and addressed several issues uncovered during a careful review:helmdriver.NewSQLper reconcile would leak a connection pool indefinitely (Helm v4 has noCloseonstorage.Driver). The newaction.SQLDriverPoolcaches drivers per storage namespace, bounding live pools to the set of namespaces actually in use rather than once per reconcile.--helm-storage-driver=sql, the controller probes the database with a transientdatabase/sql.Open+PingContext+Closebefore starting the manager, surfacing invalid DSNs or unreachable backends at boot rather than failing every HelmRelease reconcile. The probe connection is cleanly closed and does not contribute to the long-lived pool.helmdriver.NewSQL/sqlx.Connecterrors can echo the DSN. They're caught at theWithStoragecall site and replaced with a generic message so DSN material cannot leak intoHelmReleasestatus conditions.secret/secretsandconfigmap/configmapsare accepted interchangeably, matchingpkg/action.(*Configuration).Init.SetFactoryrather than a package-level mutable global, matching the existingConfigFactoryOptionidiom in this package.Use cases
Backwards compatibility
HELM_DRIVERis now consulted as a fallback (it was previously ignored).--helm-storage-driverflag value is normalised once at startup; an invalid value fails the controller process, not every reconcile.Tests
Adds coverage for:
IsSupportedStorageDriver/NormalizeStorageDriverName.go test ./internal/action -run 'TestWithStorage|TestSQLDriverPool|TestIsSupportedStorageDriver|TestConfigFactory|TestNewConfigFactory|TestWithDriver|TestWithSQLDriverPool|TestStorageLog'passes (all 30+ subtests, no envtest required).References
CC @fiscafusca (original #760 author) — thank you for the groundwork; the configuration UX in this PR follows yours.
🤖 The implementation was assisted by Claude Code; the resulting code, tests, and design decisions are my own and have been reviewed by an independent Codex pass before submission.