The string returned by the string builder was cast to a byte slice
anyways. Remove this indirection. Also, write directly to the buffer
instead of using fmt.Fprintln(...).
Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
* fix: route registry messages to stderr in template and show
When pulling an OCI chart, the registry client prints "Pulled: ..." and
"Digest: ..." status lines (and deprecation/underscore warnings) to its
configured output writer. Since v4.2.1 (introduced by #32056), these
messages leaked into the stdout output of helm template and helm show,
breaking downstream consumers such as cdk8s and other YAML parsers.
Fix by passing the command's stderr to the registry client in the
template and show commands instead of stdout. This keeps stdout clean
for machine-readable YAML while still surfacing registry warnings and
status messages on stderr for troubleshooting, rather than discarding
them. The pull/push commands continue to print these messages on their
normal output writer.
The show command's addRegistryClient writer parameter is renamed to
registryOut and wired through to the registry client, so it is no longer
a no-op.
Fixes#32215
Signed-off-by: amarkdotdev <amarkdotdev@users.noreply.github.com>
* test(cmd): cover registry client stderr routing via OCI pull
Exercise helm template and helm show against an in-process OCI registry
(repotest.NewOCIServer) and assert Pulled:/Digest: status lines appear
on stderr only, keeping stdout free of registry noise.
Signed-off-by: amarkdotdev <amarkdotdev@users.noreply.github.com>
* test(cmd): drop weak addRegistryClient unit test
OCI pull coverage already asserts Pulled/Digest land on stderr, not stdout.
Signed-off-by: amarkdotdev <amarkdotdev@users.noreply.github.com>
---------
Signed-off-by: amarkdotdev <amarkdotdev@users.noreply.github.com>
Co-authored-by: amarkdotdev <amarkdotdev@users.noreply.github.com>
* refactor: remove per-file decompression size limit
Remove MaxDecompressedFileSize as it's no longer necessary after
migrating to a maintained JSON schema library (santhosh-tekuri/jsonschema/v6).
The original limit was added to protect against vulnerabilities in an
unmaintained library.
The total decompressed chart size limit (MaxDecompressedChartSize) remains
to protect against other attack vectors.
Partially resolves#30738
Related:
- https://github.com/helm/helm/pull/30743
Signed-off-by: Benoit Tigeot <benoit.tigeot@lifen.fr>
* fix: deprecate MaxDecompressedFileSize instead of removing
As Matt suggested we should keep the variable until v5 as it can be used
because it is public.
Related:
- https://github.com/helm/helm/pull/31748#discussion_r2738518696
Signed-off-by: Benoit Tigeot <benoit.tigeot@lifen.fr>
* fix: enforce aggregate size budget on directory loading
Directory-based chart loading (`LoadDir`) used unbounded `os.ReadFile`
calls with no total size check. Archive loading already enforces
`MaxDecompressedChartSize` via a remaining-byte budget but directory
loading did not, leaving local charts and `file://` dependencies as
an unbounded memory path.
Add `ReadFileWithBudget` in the archive package and use it in both
v2 and v3 directory loaders so they track the same aggregate budget.
Ref: https://github.com/helm/helm/pull/31748#issuecomment-4138927643
Signed-off-by: Benoit Tigeot <benoit.tigeot@lifen.fr>
* fix: cap directory budget reads with LimitReader
Use `os.Open` + `io.LimitReader` instead of `os.ReadFile` in
`ReadFileWithBudget` so a file that grows between stat and read
cannot allocate unbounded memory.
Also fix `MaxDecompressedFileSize` doc comment to reflect it is
unused/deprecated, add nil guard on remaining, and check
`os.Stat` errors in tests.
Signed-off-by: Benoit Tigeot <benoit.tigeot@lifen.fr>
* test: add v3 directory loader budget test
Mirror the v2 `TestLoadDirExceedsBudget` test for the v3 loader
to prevent budget enforcement regressions in either path.
Signed-off-by: Benoit Tigeot <benoit.tigeot@lifen.fr>
* refactor(loader): make read budget configurable
Follow recommendations from https://github.com/helm/helm/pull/31748#discussion_r3058581419
Signed-off-by: Benoit Tigeot <benoit.tigeot@lifen.fr>
* fix(loader): export BudgetedReader for cross-package use
Signed-off-by: Benoit Tigeot <benoit.tigeot@lifen.fr>
* fix(loader): rename max param to avoid shadowing built-in
Signed-off-by: Benoit Tigeot <benoit.tigeot@lifen.fr>
---------
Signed-off-by: Benoit Tigeot <benoit.tigeot@lifen.fr>
Add ParseCreateOptions/ParseUpdateOptions helpers to pkg/kube so tests can
inspect resolved option values without depending on unexported structs.
FailingKubeClient records the last Create/Update options in RecordedCreateOptions
and RecordedUpdateOptions.
Update TestInstallRelease_DryRunServerValidation and
TestUpgradeRelease_DryRunServerValidation to:
- explicitly use TakeOwnership/ServerSideApply=true so the server dry-run
path is actually exercised (previously the test accidentally passed because
the ownership-check error also contained the string 'validation error')
- assert RecordedUpdateOptions.DryRun is true for --dry-run=server and
false for --dry-run=client, so removing the DryRun option from the call
site would correctly break the test
Signed-off-by: MrJack <36191829+biagiopietro@users.noreply.github.com>
Client-side update paths do not honor the dry-run flag, so validating under
SSA semantics when the real upgrade would use CSA gives misleading results.
Return an explicit error instead of silently forcing SSA.
Also use the computed serverSideApply value directly to avoid forcing SSA,
and gate UpgradeClientSideFieldManager on whether the previous release used
CSA (matching the real upgrade path).
Signed-off-by: MrJack <36191829+biagiopietro@users.noreply.github.com>
Move server-side dry-run validation to after namespace creation so that
resources inside a not-yet-existing namespace do not get a 'not found'
error when --create-namespace is also set.
Also: error explicitly when --dry-run=server is combined with
--server-side=false (client-side paths ignore the dry-run flag); harden
the ForceReplace option to false (Update rejects SSA+ForceReplace); gate
UpgradeClientSideFieldManager on TakeOwnership instead of passing true
unconditionally.
Signed-off-by: MrJack <36191829+biagiopietro@users.noreply.github.com>
Client-side update paths (patchResourceClientSide, replaceResource) do not
honor the dryRun flag, so Update(..., DryRun(true), ServerSideApply(false))
would still perform real writes. Add a guard that returns an error in the
same style as the existing forceConflicts && !serverSideApply check.
Signed-off-by: MrJack <36191829+biagiopietro@users.noreply.github.com>
This is needed for goreleaser to create updates. Its latest release
needs it.
Updates needed for linting and to fix failed tests resulting from
Go std library changes.
Signed-off-by: Matt Farina <matt.farina@suse.com>
* fix(provenance): support GnuPG keybox (pubring.kbx) keyrings
Starting with GnuPG 2.1, file-backed public keyrings can use
~/.gnupg/pubring.kbx instead of the legacy pubring.gpg. Helm only read
the legacy format, so chart and plugin verification failed on
installations using the file-backed keybox.
Make the keyring loader format-aware:
- GnuPG keybox (pubring.kbx): extract OpenPGP keyblocks from the
keybox container without adding a dependency. Skip ephemeral blobs,
matching GnuPG's own read behavior.
- ASCII-armored keyrings: load single or concatenated exports.
- Legacy binary packet streams (pubring.gpg): retain the existing path.
defaultKeyring() falls back to pubring.kbx when pubring.gpg is absent.
pubring.gpg keeps precedence when both files exist.
This change covers file-backed public keyrings. It does not read the
SQLite database used by keyboxd, which needs a separate design.
Related to #31836
Signed-off-by: Ruslan Shaydullin <shaydullin.r.d@outlook.com>
* fix(provenance): treat only not-exist as keyring absence in defaultKeyring
A stat error other than 'not exist' (e.g. a permission problem) meant
the file may well be present, but defaultKeyring() skipped past it: an
unreadable pubring.gpg silently lost precedence to pubring.kbx, and the
surfaced error could point at the wrong file. Treat only fs.ErrNotExist
as absence. For any other stat error, return that path unchanged so the
real error surfaces when the keyring is opened.
Signed-off-by: Ruslan Shaydullin <shaydullin.r.d@outlook.com>
---------
Signed-off-by: Ruslan Shaydullin <shaydullin.r.d@outlook.com>
Co-authored-by: George Jenkins <gvjenkins@gmail.com>
MrJack
dependabot[bot]
Robert Sirchia
George Jenkins
Scott Rigby
Tom Wieczorek
Matthieu MOREL
Aaron Mark
Benoit Tigeot
Matt Farina
cuishuang
MsfPablo
Ruslan Shaydullin