feat: clean null fields in rendered manifests by default for better c…

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.

pkg/action/chart_lint.go

@@ -312,15 +312,15 @@ 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)
 		}
 	}
 
 	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)
 		}

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)
 	}

pkg/action/release_install.go

@@ -423,15 +423,15 @@ 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)
 			}
 		}
 
 		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))
 	}

pkg/action/release_plan_install.go

@@ -318,15 +318,15 @@ 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)
 		}
 	}
 
 	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)
 		}

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)
 		}

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)
 		}

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)

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
 }
 

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
 }