From 1cb2e91db8dd40e53ed3cdee64b768b4284c9aea Mon Sep 17 00:00:00 2001 From: Ilya Lesikov Date: Tue, 4 Aug 2026 12:56:04 +0300 Subject: [PATCH] feat: clean null fields in rendered manifests by default for better compat Signed-off-by: Ilya Lesikov --- README.md | 13 ------------- pkg/action/chart_lint.go | 6 +++--- pkg/action/release_get.go | 2 +- pkg/action/release_install.go | 14 +++++++------- pkg/action/release_plan_install.go | 6 +++--- pkg/action/release_rollback.go | 8 ++++---- pkg/action/release_uninstall.go | 2 +- pkg/release/release.go | 5 +---- pkg/resource/spec/resource_spec.go | 6 ++---- pkg/resource/spec/unstruct.go | 5 ----- 10 files changed, 22 insertions(+), 45 deletions(-) diff --git a/README.md b/README.md index c69b0d67..b21148f3 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,6 @@ Nelm is production-ready: as the werf deployment engine, it was battle-tested ac - [`NELM_FEAT_NATIVE_RELEASE_UNINSTALL` environment variable](#nelm_feat_native_release_uninstall-environment-variable) - [`NELM_FEAT_PERIODIC_STACK_TRACES` environment variable](#nelm_feat_periodic_stack_traces-environment-variable) - [`NELM_FEAT_FIELD_SENSITIVE` environment variable](#nelm_feat_field_sensitive-environment-variable) - - [`NELM_FEAT_CLEAN_NULL_FIELDS` environment variable](#nelm_feat_clean_null_fields-environment-variable) - [`NELM_FEAT_MORE_DETAILED_EXIT_CODE_FOR_PLAN` environment variable](#nelm_feat_more_detailed_exit_code_for_plan-environment-variable) - [More documentation](#more-documentation) - [Limitations](#limitations) @@ -845,18 +844,6 @@ export NELM_FEAT_FIELD_SENSITIVE=true nelm release plan install -n myproject -r myproject ``` -### `NELM_FEAT_CLEAN_NULL_FIELDS` environment variable - -Improve Helm chart compatibility. When rendering charts, remove keys with `null` values from the rendered resource manifests, before applying them. Otherwise, SSA often fail on `null` values, which didn't happen with 3WM. - -Will be the default in the next major release. - -Example: -```shell -export NELM_FEAT_CLEAN_NULL_FIELDS=true -nelm release install -n myproject -r myproject -``` - ### `NELM_FEAT_MORE_DETAILED_EXIT_CODE_FOR_PLAN` environment variable When the `--exit-code` flag is specified for `nelm release plan install`, return exit code 3, if no resource changes planned, but release still must be installed. Previously, exit code 2 was returned in this case. diff --git a/pkg/action/chart_lint.go b/pkg/action/chart_lint.go index 3ac422e1..4d8a833c 100644 --- a/pkg/action/chart_lint.go +++ b/pkg/action/chart_lint.go @@ -312,7 +312,7 @@ func ChartLint(ctx context.Context, opts ChartLintOptions) error { var prevRelResSpecs []*spec.ResourceSpec if prevRelease != nil { - prevRelResSpecs, err = release.ReleaseToResourceSpecs(ctx, prevRelease, opts.ReleaseNamespace, false) + prevRelResSpecs, err = release.ReleaseToResourceSpecs(ctx, prevRelease, opts.ReleaseNamespace) if err != nil { return fmt.Errorf("convert previous release to resource specs: %w", err) } @@ -320,7 +320,7 @@ func ChartLint(ctx context.Context, opts ChartLintOptions) error { log.Default.Debug(ctx, "Convert new release to resource specs") - newRelResSpecs, err := release.ReleaseToResourceSpecs(ctx, newRelease, opts.ReleaseNamespace, false) + newRelResSpecs, err := release.ReleaseToResourceSpecs(ctx, newRelease, opts.ReleaseNamespace) if err != nil { return fmt.Errorf("convert new release to resource specs: %w", err) } @@ -353,7 +353,7 @@ func ChartLint(ctx context.Context, opts ChartLintOptions) error { var lastDeployedOrLastRelResSpecs []*spec.ResourceSpec if lastDeployedOrLastRelease != nil { - lastDeployedOrLastRelResSpecs, err = release.ReleaseToResourceSpecs(ctx, lastDeployedOrLastRelease, opts.ReleaseNamespace, false) + lastDeployedOrLastRelResSpecs, err = release.ReleaseToResourceSpecs(ctx, lastDeployedOrLastRelease, opts.ReleaseNamespace) if err != nil { return fmt.Errorf("convert last deployed or last release to resource specs: %w", err) } diff --git a/pkg/action/release_get.go b/pkg/action/release_get.go index 8d9afdf9..c6e24045 100644 --- a/pkg/action/release_get.go +++ b/pkg/action/release_get.go @@ -192,7 +192,7 @@ func ReleaseGet(ctx context.Context, releaseName, releaseNamespace string, opts Values: values, } - resSpecs, err := release.ReleaseToResourceSpecs(ctx, relAccessor, releaseNamespace, false) + resSpecs, err := release.ReleaseToResourceSpecs(ctx, relAccessor, releaseNamespace) if err != nil { return nil, fmt.Errorf("convert release to resource specs: %w", err) } diff --git a/pkg/action/release_install.go b/pkg/action/release_install.go index 6d2a6420..910c9650 100644 --- a/pkg/action/release_install.go +++ b/pkg/action/release_install.go @@ -423,7 +423,7 @@ func releaseInstall(ctx context.Context, ctxCancelFn context.CancelCauseFunc, re var prevRelResSpecs []*spec.ResourceSpec if prevRelease != nil { - prevRelResSpecs, err = release.ReleaseToResourceSpecs(ctx, prevRelease, releaseNamespace, false) + prevRelResSpecs, err = release.ReleaseToResourceSpecs(ctx, prevRelease, releaseNamespace) if err != nil { return fmt.Errorf("convert previous release to resource specs: %w", err) } @@ -431,7 +431,7 @@ func releaseInstall(ctx context.Context, ctxCancelFn context.CancelCauseFunc, re log.Default.Debug(ctx, "Convert new release to resource specs") - newRelResSpecs, err := release.ReleaseToResourceSpecs(ctx, newRelease, releaseNamespace, false) + newRelResSpecs, err := release.ReleaseToResourceSpecs(ctx, newRelease, releaseNamespace) if err != nil { return fmt.Errorf("convert new release to resource specs: %w", err) } @@ -463,7 +463,7 @@ func releaseInstall(ctx context.Context, ctxCancelFn context.CancelCauseFunc, re var lastDeployedOrLastRelResSpecs []*spec.ResourceSpec if lastDeployedOrLastRelease != nil { - lastDeployedOrLastRelResSpecs, err = release.ReleaseToResourceSpecs(ctx, lastDeployedOrLastRelease, releaseNamespace, false) + lastDeployedOrLastRelResSpecs, err = release.ReleaseToResourceSpecs(ctx, lastDeployedOrLastRelease, releaseNamespace) if err != nil { return fmt.Errorf("convert last deployed or last release to resource specs: %w", err) } @@ -835,7 +835,7 @@ func runRollbackPlan(ctx context.Context, releaseName, releaseNamespace string, log.Default.Debug(ctx, "Convert prev deployed release to resource specs") - resSpecs, err := release.ReleaseToResourceSpecs(ctx, prevDeployedRelease, releaseNamespace, false) + resSpecs, err := release.ReleaseToResourceSpecs(ctx, prevDeployedRelease, releaseNamespace) if err != nil { return nil, nonCritErrs, critErrs.Add(fmt.Errorf("convert previous deployed release to resource specs: %w", err)) } @@ -881,14 +881,14 @@ func runRollbackPlan(ctx context.Context, releaseName, releaseNamespace string, log.Default.Debug(ctx, "Convert failed release to resource specs") - failedRelResSpecs, err := release.ReleaseToResourceSpecs(ctx, failedRelease, releaseNamespace, false) + failedRelResSpecs, err := release.ReleaseToResourceSpecs(ctx, failedRelease, releaseNamespace) if err != nil { return nil, nonCritErrs, critErrs.Add(fmt.Errorf("convert previous release to resource specs: %w", err)) } log.Default.Debug(ctx, "Convert new release to resource specs") - newRelResSpecs, err := release.ReleaseToResourceSpecs(ctx, newRelease, releaseNamespace, false) + newRelResSpecs, err := release.ReleaseToResourceSpecs(ctx, newRelease, releaseNamespace) if err != nil { return nil, nonCritErrs, critErrs.Add(fmt.Errorf("convert new release to resource specs: %w", err)) } @@ -914,7 +914,7 @@ func runRollbackPlan(ctx context.Context, releaseName, releaseNamespace string, log.Default.Debug(ctx, "Build resource infos") - lastDeployedOrLastRelResSpecs, err := release.ReleaseToResourceSpecs(ctx, prevDeployedRelease, releaseNamespace, false) + lastDeployedOrLastRelResSpecs, err := release.ReleaseToResourceSpecs(ctx, prevDeployedRelease, releaseNamespace) if err != nil { return nil, nonCritErrs, critErrs.Add(fmt.Errorf("convert last deployed or last release to resource specs: %w", err)) } diff --git a/pkg/action/release_plan_install.go b/pkg/action/release_plan_install.go index 7cb3e7dd..819f76a6 100644 --- a/pkg/action/release_plan_install.go +++ b/pkg/action/release_plan_install.go @@ -318,7 +318,7 @@ func releasePlanInstall(ctx context.Context, ctxCancelFn context.CancelCauseFunc var prevRelResSpecs []*spec.ResourceSpec if prevRelease != nil { - prevRelResSpecs, err = release.ReleaseToResourceSpecs(ctx, prevRelease, releaseNamespace, false) + prevRelResSpecs, err = release.ReleaseToResourceSpecs(ctx, prevRelease, releaseNamespace) if err != nil { return nil, fmt.Errorf("convert previous release to resource specs: %w", err) } @@ -326,7 +326,7 @@ func releasePlanInstall(ctx context.Context, ctxCancelFn context.CancelCauseFunc log.Default.Debug(ctx, "Convert new release to resource specs") - newRelResSpecs, err := release.ReleaseToResourceSpecs(ctx, newRelease, releaseNamespace, false) + newRelResSpecs, err := release.ReleaseToResourceSpecs(ctx, newRelease, releaseNamespace) if err != nil { return nil, fmt.Errorf("convert new release to resource specs: %w", err) } @@ -355,7 +355,7 @@ func releasePlanInstall(ctx context.Context, ctxCancelFn context.CancelCauseFunc var lastDeployedOrLastRelResSpecs []*spec.ResourceSpec if lastDeployedOrLastRelease != nil { - lastDeployedOrLastRelResSpecs, err = release.ReleaseToResourceSpecs(ctx, lastDeployedOrLastRelease, releaseNamespace, false) + lastDeployedOrLastRelResSpecs, err = release.ReleaseToResourceSpecs(ctx, lastDeployedOrLastRelease, releaseNamespace) if err != nil { return nil, fmt.Errorf("convert last deployed or last release to resource specs: %w", err) } diff --git a/pkg/action/release_rollback.go b/pkg/action/release_rollback.go index 8d41255f..ab01f6fb 100644 --- a/pkg/action/release_rollback.go +++ b/pkg/action/release_rollback.go @@ -230,7 +230,7 @@ func releaseRollback(ctx context.Context, ctxCancelFn context.CancelCauseFunc, r log.Default.Debug(ctx, "Convert release to resource specs") - rollbackReleaseResSpecs, err := release.ReleaseToResourceSpecs(ctx, rollbackRelease, releaseNamespace, false) + rollbackReleaseResSpecs, err := release.ReleaseToResourceSpecs(ctx, rollbackRelease, releaseNamespace) if err != nil { return fmt.Errorf("convert release to rollback to resource specs: %w", err) } @@ -275,14 +275,14 @@ func releaseRollback(ctx context.Context, ctxCancelFn context.CancelCauseFunc, r log.Default.Debug(ctx, "Convert previous release to resource specs") - prevRelResSpecs, err := release.ReleaseToResourceSpecs(ctx, prevRelease, releaseNamespace, false) + prevRelResSpecs, err := release.ReleaseToResourceSpecs(ctx, prevRelease, releaseNamespace) if err != nil { return fmt.Errorf("convert previous release to resource specs: %w", err) } log.Default.Debug(ctx, "Convert new release to resource specs") - newRelResSpecs, err := release.ReleaseToResourceSpecs(ctx, newRelease, releaseNamespace, false) + newRelResSpecs, err := release.ReleaseToResourceSpecs(ctx, newRelease, releaseNamespace) if err != nil { return fmt.Errorf("convert new release to resource specs: %w", err) } @@ -312,7 +312,7 @@ func releaseRollback(ctx context.Context, ctxCancelFn context.CancelCauseFunc, r var lastDeployedOrLastRelResSpecs []*spec.ResourceSpec if lastDeployedOrLastRelease != nil { - lastDeployedOrLastRelResSpecs, err = release.ReleaseToResourceSpecs(ctx, lastDeployedOrLastRelease, releaseNamespace, false) + lastDeployedOrLastRelResSpecs, err = release.ReleaseToResourceSpecs(ctx, lastDeployedOrLastRelease, releaseNamespace) if err != nil { return fmt.Errorf("convert last deployed or last release to resource specs: %w", err) } diff --git a/pkg/action/release_uninstall.go b/pkg/action/release_uninstall.go index f3219b52..1b1b42ca 100644 --- a/pkg/action/release_uninstall.go +++ b/pkg/action/release_uninstall.go @@ -214,7 +214,7 @@ func releaseUninstall(ctx context.Context, ctxCancelFn context.CancelCauseFunc, log.Default.Debug(ctx, "Convert previous release to resource specs") - prevRelResSpecs, err := release.ReleaseToResourceSpecs(ctx, prevRelease, releaseNamespace, false) + prevRelResSpecs, err := release.ReleaseToResourceSpecs(ctx, prevRelease, releaseNamespace) if err != nil { return fmt.Errorf("convert previous release to resource specs: %w", err) } diff --git a/pkg/release/release.go b/pkg/release/release.go index 43672ed3..f0890ca1 100644 --- a/pkg/release/release.go +++ b/pkg/release/release.go @@ -260,12 +260,11 @@ func NewRelease(name, namespace string, revision int, deployType common.DeployTy } // Constructs ResourceSpecs from a Release object. -func ReleaseToResourceSpecs(ctx context.Context, rel helmrel.Accessor, releaseNamespace string, noCleanNullFields bool) ([]*spec.ResourceSpec, error) { +func ReleaseToResourceSpecs(ctx context.Context, rel helmrel.Accessor, releaseNamespace string) ([]*spec.ResourceSpec, error) { var resources []*spec.ResourceSpec for _, manifest := range util.SplitManifests(rel.UnstoredManifest()) { if res, err := spec.NewResourceSpecFromManifest(ctx, manifest, releaseNamespace, spec.ResourceSpecOptions{ StoreAs: common.StoreAsNone, - LegacyNoCleanNullFields: noCleanNullFields, DropInvalidAnnotationsAndLabels: true, }); err != nil { return nil, fmt.Errorf("construct resource spec from unstored manifest: %w", err) @@ -277,7 +276,6 @@ func ReleaseToResourceSpecs(ctx context.Context, rel helmrel.Accessor, releaseNa for _, manifest := range util.SplitManifests(rel.Manifest()) { if res, err := spec.NewResourceSpecFromManifest(ctx, manifest, releaseNamespace, spec.ResourceSpecOptions{ StoreAs: common.StoreAsRegular, - LegacyNoCleanNullFields: noCleanNullFields, DropInvalidAnnotationsAndLabels: true, }); err != nil { return nil, fmt.Errorf("construct resource spec from regular manifest: %w", err) @@ -294,7 +292,6 @@ func ReleaseToResourceSpecs(ctx context.Context, rel helmrel.Accessor, releaseNa if res, err := spec.NewResourceSpecFromManifest(ctx, hookAcc.Manifest(), releaseNamespace, spec.ResourceSpecOptions{ StoreAs: common.StoreAsHook, - LegacyNoCleanNullFields: noCleanNullFields, DropInvalidAnnotationsAndLabels: true, }); err != nil { return nil, fmt.Errorf("construct resource spec from hook manifest: %w", err) diff --git a/pkg/resource/spec/resource_spec.go b/pkg/resource/spec/resource_spec.go index ef26405a..99100664 100644 --- a/pkg/resource/spec/resource_spec.go +++ b/pkg/resource/spec/resource_spec.go @@ -23,9 +23,8 @@ type ResourceSpec struct { } func NewResourceSpec(unstruct *unstructured.Unstructured, releaseNamespace string, opts ResourceSpecOptions) *ResourceSpec { - unstruct = CleanUnstruct(unstruct, CleanUnstructOptions{ - CleanNullFields: !opts.LegacyNoCleanNullFields, - }) + unstruct = unstruct.DeepCopy() + unstruct.Object = cleanNulls(unstruct.Object).(map[string]interface{}) if opts.StoreAs == "" { if IsHook(unstruct.GetAnnotations()) { @@ -88,7 +87,6 @@ func (s *ResourceSpec) SetLabels(labels map[string]string) { type ResourceSpecOptions struct { DropInvalidAnnotationsAndLabels bool FilePath string - LegacyNoCleanNullFields bool // TODO(major): always clean StoreAs common.StoreAs } diff --git a/pkg/resource/spec/unstruct.go b/pkg/resource/spec/unstruct.go index b9cd77d8..6ba715e3 100644 --- a/pkg/resource/spec/unstruct.go +++ b/pkg/resource/spec/unstruct.go @@ -14,7 +14,6 @@ type CleanUnstructOptions struct { CleanHelmShAnnos bool CleanLabels map[string]string CleanManagedFields bool - CleanNullFields bool CleanReleaseAnnosLabels bool CleanRuntimeData bool CleanWerfIoAnnos bool @@ -71,10 +70,6 @@ func CleanUnstruct(unstruct *unstructured.Unstructured, opts CleanUnstructOption unstructCopy.SetLabels(filteredLabels) } - if opts.CleanNullFields { - unstructCopy.Object = cleanNulls(unstructCopy.Object).(map[string]interface{}) - } - return unstructCopy }