diff --git a/pkg/action/install.go b/pkg/action/install.go index 605c423bc..8647f2481 100644 --- a/pkg/action/install.go +++ b/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 } } diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index 2d83abe27..30b69b6c0 100644 --- a/pkg/action/install_test.go +++ b/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") +} diff --git a/pkg/kube/client.go b/pkg/kube/client.go index ba881c560..b3c48904d 100644 --- a/pkg/kube/client.go +++ b/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{ - 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 { + createOptions, err := resolveClientCreateOptions(options...) + if err != nil { return nil, fmt.Errorf("invalid client create option(s): %w", err) } diff --git a/pkg/kube/client_test.go b/pkg/kube/client_test.go index 86574a3e5..19f5f8096 100644 --- a/pkg/kube/client_test.go +++ b/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) + }) +}