fix(action): do not overwrite an existing namespace with --create-namespace

--create-namespace is documented as creating the release namespace only when it
is not present, and the namespace is not part of the release. It was created
with the same apply method as the chart resources, so with server-side apply
Helm sent an apply patch for a minimal Namespace object. On a namespace that
already exists that patch makes Helm a field manager of a namespace it does not
own and strips the labels and annotations set by whoever created it, for
example a namespace deployed by another chart.

The namespace is now always created client-side, so an existing namespace
answers AlreadyExists and is left untouched, regardless of --server-side.

ResolveClientCreateOptions is added to pkg/kube so that code wrapping or faking
Interface.Create can see how the resources it was handed are meant to be
created; Client.Create now resolves its options through the same helper.

Closes #31767

Signed-off-by: ChadiDridi <dridichady@gmail.com>

pkg/action/install.go

@@ -447,9 +447,16 @@ func (i *Install) RunWithContext(ctx context.Context, ch ci.Charter, vals map[st
 			return nil, err
 		}
 
+		// The namespace is not part of the release, it is only a precondition for
+		// installing into it, and --create-namespace is documented as creating it
+		// only when it is not present. A client-side create is used so that an
+		// existing namespace reports AlreadyExists and is left untouched. A
+		// server-side apply would instead make Helm a field manager of a
+		// namespace it does not own and drop labels and annotations set by
+		// whoever created it.
 		if _, err := i.cfg.KubeClient.Create(
 			resourceList,
-			kube.ClientCreateOptionServerSideApply(i.ServerSideApply, false)); err != nil && !apierrors.IsAlreadyExists(err) {
+			kube.ClientCreateOptionServerSideApply(false, false)); err != nil && !apierrors.IsAlreadyExists(err) {
 			return nil, err
 		}
 	}

pkg/action/install_test.go

@@ -35,6 +35,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	appsv1 "k8s.io/api/apps/v1"
+	v1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -43,6 +44,7 @@ import (
 	"k8s.io/cli-runtime/pkg/resource"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest/fake"
+	"sigs.k8s.io/yaml"
 
 	ci "helm.sh/helm/v4/pkg/chart"
 
@@ -1253,3 +1255,78 @@ func TestInstallRelease_WaitOptionsPassedDownstream(t *testing.T) {
 	// Verify that WaitOptions were passed to GetWaiter
 	is.NotEmpty(failer.RecordedWaitOptions, "WaitOptions should be passed to GetWaiter")
 }
+
+// namespaceRecordingKubeClient records the resources passed to Create and
+// answers with AlreadyExists for the namespace, the way the API server answers
+// a client-side create for a namespace that is already there.
+type namespaceRecordingKubeClient struct {
+	kubefake.PrintingKubeClient
+	createdNamespaces   []string
+	namespaceCreateOpts kube.ClientCreateOptions
+}
+
+// Build returns a resource for the namespace manifest so that the namespace
+// reaches Create; everything else keeps the printing client behaviour.
+func (c *namespaceRecordingKubeClient) Build(r io.Reader, validate bool) (kube.ResourceList, error) {
+	manifest, err := io.ReadAll(r)
+	if err != nil {
+		return nil, err
+	}
+	if !strings.Contains(string(manifest), "kind: Namespace") {
+		return c.PrintingKubeClient.Build(bytes.NewReader(manifest), validate)
+	}
+
+	var ns v1.Namespace
+	if err := yaml.Unmarshal(manifest, &ns); err != nil {
+		return nil, err
+	}
+	return kube.ResourceList{{
+		Name: ns.Name,
+		Mapping: &meta.RESTMapping{
+			GroupVersionKind: schema.GroupVersionKind{Version: "v1", Kind: "Namespace"},
+		},
+		Object: &ns,
+	}}, nil
+}
+
+func (c *namespaceRecordingKubeClient) Create(resources kube.ResourceList, options ...kube.ClientCreateOption) (*kube.Result, error) {
+	for _, r := range resources {
+		if r.Mapping != nil && r.Mapping.GroupVersionKind.Kind == "Namespace" {
+			resolved, err := kube.ResolveClientCreateOptions(options...)
+			if err != nil {
+				return nil, err
+			}
+			c.createdNamespaces = append(c.createdNamespaces, r.Name)
+			c.namespaceCreateOpts = resolved
+			return nil, apierrors.NewAlreadyExists(schema.GroupResource{Resource: "namespaces"}, r.Name)
+		}
+	}
+	return c.PrintingKubeClient.Create(resources, options...)
+}
+
+// TestInstallReleaseWithCreateNamespaceExisting checks that installing into a
+// namespace that already exists succeeds and that Helm only ever asks for the
+// namespace to be created, so an existing namespace keeps the labels and
+// annotations set by whoever created it.
+func TestInstallReleaseWithCreateNamespaceExisting(t *testing.T) {
+	is := assert.New(t)
+	req := require.New(t)
+
+	config := actionConfigFixture(t)
+	recorder := &namespaceRecordingKubeClient{PrintingKubeClient: kubefake.PrintingKubeClient{Out: io.Discard}}
+	config.KubeClient = recorder
+
+	instAction := installActionWithConfig(config)
+	instAction.CreateNamespace = true
+
+	res, err := instAction.Run(buildChart(), map[string]any{})
+	req.NoError(err, "install into an existing namespace should succeed")
+
+	rel, err := releaserToV1Release(res)
+	req.NoError(err)
+	is.Equal("spaced", rel.Namespace)
+	is.Equal([]string{"spaced"}, recorder.createdNamespaces)
+	// A server-side apply would make Helm a field manager of the namespace and
+	// drop the labels and annotations it does not set itself.
+	is.False(recorder.namespaceCreateOpts.ServerSideApply, "the namespace must be created client-side")
+}

pkg/kube/client.go

@@ -275,6 +275,49 @@ type clientCreateOptions struct {
 
 type ClientCreateOption func(*clientCreateOptions) error
 
+// ClientCreateOptions reports the settings a list of ClientCreateOption
+// resolves to. It lets callers that wrap or fake Interface.Create see how the
+// resources they were handed are meant to be created.
+type ClientCreateOptions struct {
+	ServerSideApply          bool
+	ForceConflicts           bool
+	DryRun                   bool
+	FieldValidationDirective FieldValidationDirective
+}
+
+// ResolveClientCreateOptions applies options on top of the defaults used by
+// Client.Create and returns the result.
+func ResolveClientCreateOptions(options ...ClientCreateOption) (ClientCreateOptions, error) {
+	resolved, err := resolveClientCreateOptions(options...)
+	if err != nil {
+		return ClientCreateOptions{}, err
+	}
+
+	return ClientCreateOptions{
+		ServerSideApply:          resolved.serverSideApply,
+		ForceConflicts:           resolved.forceConflicts,
+		DryRun:                   resolved.dryRun,
+		FieldValidationDirective: resolved.fieldValidationDirective,
+	}, nil
+}
+
+func resolveClientCreateOptions(options ...ClientCreateOption) (clientCreateOptions, error) {
+	createOptions := clientCreateOptions{
+		serverSideApply:          true, // Default to server-side apply
+		fieldValidationDirective: FieldValidationDirectiveStrict,
+	}
+
+	errs := make([]error, 0, len(options))
+	for _, o := range options {
+		errs = append(errs, o(&createOptions))
+	}
+	if err := errors.Join(errs...); err != nil {
+		return clientCreateOptions{}, err
+	}
+
+	return createOptions, nil
+}
+
 // ClientCreateOptionServerSideApply enables performing object apply server-side
 // see: https://kubernetes.io/docs/reference/using-api/server-side-apply/
 //
@@ -353,16 +396,8 @@ func (c *Client) makeCreateApplyFunc(serverSideApply, forceConflicts, dryRun boo
 func (c *Client) Create(resources ResourceList, options ...ClientCreateOption) (*Result, error) {
 	c.Logger().Debug("creating resource(s)", "resources", len(resources))
 
-	createOptions := clientCreateOptions{
+	createOptions, err := resolveClientCreateOptions(options...)
-		serverSideApply:          true, // Default to server-side apply
+	if err != nil {
-		fieldValidationDirective: FieldValidationDirectiveStrict,
-	}
-
-	errs := make([]error, 0, len(options))
-	for _, o := range options {
-		errs = append(errs, o(&createOptions))
-	}
-	if err := errors.Join(errs...); err != nil {
 		return nil, fmt.Errorf("invalid client create option(s): %w", err)
 	}
 

pkg/kube/client_test.go

@@ -2272,3 +2272,31 @@ func createManifest(t *testing.T, manifest string,
 	require.NoError(t, err)
 	require.NoError(t, fakeClient.Tracker().Create(mapping.Resource, obj, obj.GetNamespace()))
 }
+
+func TestResolveClientCreateOptions(t *testing.T) {
+	t.Run("defaults", func(t *testing.T) {
+		opts, err := ResolveClientCreateOptions()
+		require.NoError(t, err)
+		assert.True(t, opts.ServerSideApply)
+		assert.False(t, opts.ForceConflicts)
+		assert.False(t, opts.DryRun)
+		assert.Equal(t, FieldValidationDirectiveStrict, opts.FieldValidationDirective)
+	})
+
+	t.Run("options are applied", func(t *testing.T) {
+		opts, err := ResolveClientCreateOptions(
+			ClientCreateOptionServerSideApply(false, false),
+			ClientCreateOptionDryRun(true),
+			ClientCreateOptionFieldValidationDirective(FieldValidationDirectiveIgnore),
+		)
+		require.NoError(t, err)
+		assert.False(t, opts.ServerSideApply)
+		assert.True(t, opts.DryRun)
+		assert.Equal(t, FieldValidationDirectiveIgnore, opts.FieldValidationDirective)
+	})
+
+	t.Run("invalid combination", func(t *testing.T) {
+		_, err := ResolveClientCreateOptions(ClientCreateOptionServerSideApply(false, true))
+		require.Error(t, err)
+	})
+}