Merge 3bff902d8f into 62c7e845d1

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

@@ -2271,3 +2271,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)
+	})
+}