feat: add renderPatches support for --patches/patches.yaml - #696
Merged
Conversation
Signed-off-by: Dmitry Mordvinov <dmitry.mordvinov@flant.com>
Signed-off-by: Dmitry Mordvinov <dmitry.mordvinov@flant.com>
… re-derived ResMeta after patching Signed-off-by: Dmitry Mordvinov <dmitry.mordvinov@flant.com>
Signed-off-by: Dmitry Mordvinov <dmitry.mordvinov@flant.com>
Signed-off-by: Dmitry Mordvinov <dmitry.mordvinov@flant.com>
Signed-off-by: Dmitry Mordvinov <dmitry.mordvinov@flant.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
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
Adds a second kind of rule to
patches.yaml/--patches:renderPatches, applied to the renderedresources right after the chart is rendered. Unlike
diffPatches, which only normalize the live anddry-apply objects before they are compared, render patches change what goes into the release and to
the cluster — a way to fix up a resource you do not own (a subchart's manifest, a field a controller
fights over, metadata a policy engine demands) without forking templates.
Key changes
renderPatchestop-level key, next todiffPatches, in both chart-shippedpatches.yamland--patchesfiles: same rule shape (match,type: jq,patch), same chart-subtree scoping forchart-shipped rules, same "jq program must return exactly one object" contract.
spec.BuildRenderPatchedResourceSpecs, placed after list expansion and beforethe existing patchers, so rules match real resources and nelm's own metadata lands on top of user
patches. Semantics specific to render patches:
apiVersion/kind/name/namespace), and outputthat is not a resource at all is rejected outright;
metadata.annotationsandmetadata.labelsin the output must be string maps, the same check themanifest decoder does: a non-string value (
.metadata.annotations["werf.io/weight"] = 10) isrejected instead of being silently dropped by the apimachinery accessors, which would leave
ResourceMetawithout the annotations while the object keeps the bad value;StoreAsis re-derived from the patched object, so adding or removinghelm.sh/hookin a renderpatch really does reclassify the resource.
StoreAsNone, carried by standalone CRDs, is exemptand survives patching — it is not releasable at all, and without the exemption a single render
rule anywhere would turn every standalone CRD into a released, release-owned resource;
can key off a label or annotation an earlier rule added.
ApplyPatchesis deliberately notreused for this: its matchers must keep seeing the unpatched resource, so that diff patches take
the same rule path for the live and the dry-apply object.
DiffPatch→Patch,CompiledDiffPatch→CompiledPatch,ApplyDiffPatches→ApplyPatches,diff_patch.go→patches.go): matching, compiling andtransforming were never diff-specific, only the point of application is.
CompiledPatch.transformruns gojq viaRunWithContext, andApplyPatchestakes acontext.Context, threaded from the operation context in both the renderstage and the live/dry-apply diff paths — a looping patch (
def f: f; f) now aborts on Ctrl-C orcommand timeout with
context.Canceledinstead of hanging nelm. Matters more with this PR, sincechart render/chart lintnow execute user jq locally.resolvePatchescollects chart-shipped and file rules and compilesboth kinds immediately, so a bad regexp or jq program fails before anything is written into a
release or sent to the cluster.
plan.BuildResourceInfosOptions.DiffPatchesnow takes compiledpatches and the compile step inside
BuildResourceInfosis gone.release install,release plan install,chart renderandchart lint.release rollback,release uninstalland the rollback-on-failure path rendernothing, so they use diff patches only and ignore render rules silently instead of erroring — the
rules were already baked into the stored release, and the same patches file has to work for every
command.
--patchesand--no-default-patchesadded tochart renderandchart lint, which had no patchflags at all;
chart lintnow also honours diff patches, which it silently dropped before.docs/reference.mdregenerated; existing patch tests moved to the new names andextended to cover both kinds; new
patches_ai_test.gocovers matching, chart scope, namespacehandling, identity rejection,
StoreAsre-derivation and theStoreAsNoneexemption, chaining bypatched metadata, non-string annotations/labels, and contract violations, plus cancellation of an
infinite jq program through
Transform,ApplyPatchesandBuildRenderPatchedResourceSpecs.Why
diffPatchescan only hide drift — there was no supported way to adjust a rendered manifest, so theoptions were forking the subchart or post-processing YAML outside nelm. Render patches keep that fix
in the chart (or in a CI-supplied patches file), reuse the matcher and jq machinery
diffPatchesalready ships, and stay visible in
nelm chart renderoutput.Verification
chart renderin local mode (no cluster):chart-shipped
renderPatchesdroppedspec.replicasand added a label in the rendered output,--no-default-patchesignored them, a rule from--patchesapplied, andhelm.sh/hookadded by arender patch showed up in the manifest.
.metadata.name = "renamed"), output that is not aresource (
.spec,.spec.template.spec.containers[]),empty,., .,.metadata.name, and anunparsable jq program — the last fails at resolve time, before any rendered result is used.
task test:unit(full suite) plustask test:unit tags=ai_tests paths="./pkg/resource/spec"forthe new tagged tests.
golangci-lint run <pkg dirs except helm> ./cmd/...), 0 issues.Worth knowing: on macOS
task lintsilently narrows to./cmd/..., because the Taskfile's defaultpath expression relies on GNU
find -printf.StoreAsNoneexemption, metadata string-map check, per-rule matcher re-derivation)have no hand-run of their own beyond unit tests. The live-cluster run below is on the PR head
commit, so it exercises the render path with those fixes in place, but does not target them.
renderPatchesrule over aConfigMap:
chart renderapplied the rule,--no-default-patchesignored it, and a rule from--patcheslayered on top of the chart-shipped one.release installput the patcheddatainthe cluster; editing the rule made
release plan installshow the expected-/+diff and thenext
release installperformed anUpdate;release uninstallremoved the resource.release rollbackand the rollback-on-failure path were never exercised against acluster, so that wiring is covered by unit tests only.
Review focus / risks
diffPatchesis unreleased (it exists only on2,in no tag), so there is no user-facing break, but
plan.BuildResourceInfosOptions.DiffPatcheschanging to
[]*spec.CompiledPatchis an API change for embedders.namespaces:matches differently at render time on purpose: there is no live object, so an emptynamespace is treated as the release namespace and cluster-scoped resources are indistinguishable
from namespaced ones without an explicit namespace. Combine
namespaces:withkinds:when thatmatters.
pkg/resource/spec/resource_spec.gocarries the reasoning.StoreAsis re-derived only in the render-patch stage, deliberately: the sharedBuildPatchedResourceSpecsloop is untouched, so--add-annotation helm.sh/hook=...still does notreclassify a resource while a render patch does.
by the identity check, the one-object contract and the metadata string-map check; not guarded
against value-level mistakes elsewhere in the object, e.g. jq division yields a float and
.spec.replicas /= 2renders1.5for the API to reject.chart lintstarts applying diff patches, which changes its dry-run diff results for charts thatship
patches.yaml.