msb-public
/
helm
mirror of https://github.com/helm/helm
1
0
Code Issues Projects Releases Wiki Activity

refactor(action): return []byte from renderResources

Browse Source 
renderResources returned *bytes.Buffer, but every caller only ever read
it — .String() in install/upgrade/tests, .Bytes() once in upgrade for
validateManifest (which takes []byte natively). Internally b was used
purely as an io.Writer (five fmt.Fprintf sites, no Grow/Reset/WriteTo).

The *bytes.Buffer type bought nothing on either side; it only forced a
dead "if manifestDoc != nil" guard in install (renderResources always
returned a non-nil buffer, so the guard was always true) and a
string->buffer round-trip (install did bytes.NewBufferString(rel.Manifest)
to feed KubeClient.Build, despite having just held the bytes).

Return []byte instead: b is a []byte written via fmt.Appendf, returns
stay "return hs, b, ...", callers take the bytes directly. install
passes bytes.NewReader(manifest) to Build (no round-trip) and upgrade
hands the slice straight to validateManifest.

Pure mechanical refactor, no behavior change. pkg/action and pkg/cmd
tests pass.

Signed-off-by: 胡玮文 <huweiwen.hww@alibaba-inc.com>
pull/32683/head
胡玮文 1 week ago
parent d31cd6992f
commit 0a4b962d4e
4 changed files with 19 additions and 21 deletions
Show Stats Download Patch File Download Diff File

14
pkg/action/action.go
Unescape View File

@ -276,9 +276,9 @@ func splitAndDeannotate(postrendered, fallbackPrefix string) (map[string]string,
// TODO: As part of the refactor the duplicate code in cmd/helm/template.go should be removed // TODO: As part of the refactor the duplicate code in cmd/helm/template.go should be removed
// //
// This code has to do with writing files to disk. // This code has to do with writing files to disk.
func (cfg *Configuration) renderResources(ctx context.Context, ch *chart.Chart, values common.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, pr postrenderer.PostRenderer, interactWithRemote, enableDNS, hideSecret bool, postRenderStrategy PostRenderStrategy) ([]*release.Hook, *bytes.Buffer, string, error) { func (cfg *Configuration) renderResources(ctx context.Context, ch *chart.Chart, values common.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, pr postrenderer.PostRenderer, interactWithRemote, enableDNS, hideSecret bool, postRenderStrategy PostRenderStrategy) ([]*release.Hook, []byte, string, error) {
var hs []*release.Hook var hs []*release.Hook
b := bytes.NewBuffer(nil) var b []byte
caps, err := cfg.getCapabilities() caps, err := cfg.getCapabilities()
if err != nil { if err != nil {
@ -355,7 +355,7 @@ func (cfg *Configuration) renderResources(ctx context.Context, ch *chart.Chart,
if strings.TrimSpace(content) == "" { if strings.TrimSpace(content) == "" {
continue continue
} }
fmt.Fprintf(b, "---\n# Source: %s\n%s\n", name, content) b = fmt.Appendf(b, "---\n# Source: %s\n%s\n", name, content)
} }
return hs, b, "", err return hs, b, "", err
} }
@ -473,7 +473,7 @@ func (cfg *Configuration) renderResources(ctx context.Context, ch *chart.Chart,
if strings.TrimSpace(content) == "" { if strings.TrimSpace(content) == "" {
continue continue
} }
fmt.Fprintf(b, "---\n# Source: %s\n%s\n", name, content) b = fmt.Appendf(b, "---\n# Source: %s\n%s\n", name, content)
} }
return hs, b, "", err return hs, b, "", err
} }
@ -484,7 +484,7 @@ func (cfg *Configuration) renderResources(ctx context.Context, ch *chart.Chart,
if includeCrds { if includeCrds {
for _, crd := range ch.CRDObjects() { for _, crd := range ch.CRDObjects() {
if outputDir == "" { if outputDir == "" {
fmt.Fprintf(b, "---\n# Source: %s\n%s\n", crd.Filename, string(crd.File.Data)) b = fmt.Appendf(b, "---\n# Source: %s\n%s\n", crd.Filename, string(crd.File.Data))
} else { } else {
err = writeToFile(outputDir, crd.Filename, string(crd.File.Data), fileWritten[crd.Filename]) err = writeToFile(outputDir, crd.Filename, string(crd.File.Data), fileWritten[crd.Filename])
if err != nil { if err != nil {
@ -498,9 +498,9 @@ func (cfg *Configuration) renderResources(ctx context.Context, ch *chart.Chart,
for _, m := range manifests { for _, m := range manifests {
if outputDir == "" { if outputDir == "" {
if hideSecret && m.Head.Kind == "Secret" && m.Head.Version == "v1" { if hideSecret && m.Head.Kind == "Secret" && m.Head.Version == "v1" {
fmt.Fprintf(b, "---\n# Source: %s\n# HIDDEN: The Secret output has been suppressed\n", m.Name) b = fmt.Appendf(b, "---\n# Source: %s\n# HIDDEN: The Secret output has been suppressed\n", m.Name)
} else { } else {
fmt.Fprintf(b, "---\n# Source: %s\n%s\n", m.Name, m.Content) b = fmt.Appendf(b, "---\n# Source: %s\n%s\n", m.Name, m.Content)
} }
} else { } else {
newDir := outputDir newDir := outputDir

10
pkg/action/action_test.go
Unescape View File

@ -1845,7 +1845,7 @@ data:
name: value name: value
` `
assert.Equal(t, expectedBuf, buf.String()) assert.Equal(t, expectedBuf, string(buf))
assert.Len(t, hooks, 1) assert.Len(t, hooks, 1)
assert.Equal(t, expectedHook, hooks[0].Manifest) assert.Equal(t, expectedHook, hooks[0].Manifest)
} }
@ -1941,7 +1941,7 @@ func TestRenderResources_PostRenderer_Integration(t *testing.T) {
assert.Empty(t, notes) // Notes should be empty for this test assert.Empty(t, notes) // Notes should be empty for this test
// Verify that the post-renderer modifications are present in the output // Verify that the post-renderer modifications are present in the output
output := buf.String() output := string(buf)
expected := `--- expected := `---
# Source: hello/templates/goodbye # Source: hello/templates/goodbye
goodbye: world goodbye: world
@ -2036,8 +2036,8 @@ spec:
require.NoError(t, err) require.NoError(t, err)
assert.Len(t, hooks, 1) assert.Len(t, hooks, 1)
assert.Equal(t, "my-app", hooks[0].Name) assert.Equal(t, "my-app", hooks[0].Name)
assert.Contains(t, buf.String(), "kind: Deployment") assert.Contains(t, string(buf), "kind: Deployment")
assert.Contains(t, buf.String(), "kind: ServiceAccount") assert.Contains(t, string(buf), "kind: ServiceAccount")
} }
func TestRenderResources_PostRenderer_CombinedInvokesOnceWithEverything(t *testing.T) { func TestRenderResources_PostRenderer_CombinedInvokesOnceWithEverything(t *testing.T) {
@ -2221,7 +2221,7 @@ metadata:
// Hooks still round-trip through the release so they can execute. // Hooks still round-trip through the release so they can execute.
require.Len(t, hooks, 1) require.Len(t, hooks, 1)
assert.Contains(t, hooks[0].Manifest, "hook-cm") assert.Contains(t, hooks[0].Manifest, "hook-cm")
assert.Contains(t, manifestDoc.String(), "template-cm") assert.Contains(t, string(manifestDoc), "template-cm")
} }
func TestRenderResources_PostRenderer_NoHooksWithOnlyHooks(t *testing.T) { func TestRenderResources_PostRenderer_NoHooksWithOnlyHooks(t *testing.T) {

10
pkg/action/install.go
Unescape View File

@ -374,12 +374,10 @@ func (i *Install) RunWithContext(ctx context.Context, ch ci.Charter, vals map[st
rel := i.createRelease(chrt, vals, i.Labels) rel := i.createRelease(chrt, vals, i.Labels)
var manifestDoc *bytes.Buffer var manifest []byte
rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResources(ctx, chrt, valuesToRender, i.ReleaseName, i.OutputDir, i.SubNotes, i.UseReleaseName, i.IncludeCRDs, i.PostRenderer, interactWithServer(i.DryRunStrategy), i.EnableDNS, i.HideSecret, i.PostRenderStrategy) rel.Hooks, manifest, rel.Info.Notes, err = i.cfg.renderResources(ctx, chrt, valuesToRender, i.ReleaseName, i.OutputDir, i.SubNotes, i.UseReleaseName, i.IncludeCRDs, i.PostRenderer, interactWithServer(i.DryRunStrategy), i.EnableDNS, i.HideSecret, i.PostRenderStrategy)
// Even for errors, attach this if available // Even for errors, attach this if available
if manifestDoc != nil { rel.Manifest = string(manifest)
rel.Manifest = manifestDoc.String()
}
// Check error from render // Check error from render
if err != nil { if err != nil {
rel.SetStatus(rcommon.StatusFailed, "failed to render resource: "+err.Error()) rel.SetStatus(rcommon.StatusFailed, "failed to render resource: "+err.Error())
@ -391,7 +389,7 @@ func (i *Install) RunWithContext(ctx context.Context, ch ci.Charter, vals map[st
rel.SetStatus(rcommon.StatusPendingInstall, "Initial install underway") rel.SetStatus(rcommon.StatusPendingInstall, "Initial install underway")
var toBeAdopted kube.ResourceList var toBeAdopted kube.ResourceList
resources, err := i.cfg.KubeClient.Build(bytes.NewBufferString(rel.Manifest), !i.DisableOpenAPIValidation) resources, err := i.cfg.KubeClient.Build(bytes.NewReader(manifest), !i.DisableOpenAPIValidation)
if err != nil { if err != nil {
return nil, fmt.Errorf("unable to build kubernetes objects from release manifest: %w", err) return nil, fmt.Errorf("unable to build kubernetes objects from release manifest: %w", err)
} }

6
pkg/action/upgrade.go
Unescape View File

@ -299,7 +299,7 @@ func (u *Upgrade) prepareUpgrade(ctx context.Context, name string, chart *chartv
return nil, nil, false, err return nil, nil, false, err
} }
hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(ctx, chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer, interactWithServer(u.DryRunStrategy), u.EnableDNS, u.HideSecret, u.PostRenderStrategy) hooks, manifest, notesTxt, err := u.cfg.renderResources(ctx, chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer, interactWithServer(u.DryRunStrategy), u.EnableDNS, u.HideSecret, u.PostRenderStrategy)
if err != nil { if err != nil {
return nil, nil, false, err return nil, nil, false, err
} }
@ -328,7 +328,7 @@ func (u *Upgrade) prepareUpgrade(ctx context.Context, name string, chart *chartv
Description: "Preparing upgrade", // This should be overwritten later. Description: "Preparing upgrade", // This should be overwritten later.
}, },
Version: revision, Version: revision,
Manifest: manifestDoc.String(), Manifest: string(manifest),
Hooks: hooks, Hooks: hooks,
Labels: mergeCustomLabels(lastRelease.Labels, u.Labels), Labels: mergeCustomLabels(lastRelease.Labels, u.Labels),
ApplyMethod: string(determineReleaseSSApplyMethod(serverSideApply)), ApplyMethod: string(determineReleaseSSApplyMethod(serverSideApply)),
@ -337,7 +337,7 @@ func (u *Upgrade) prepareUpgrade(ctx context.Context, name string, chart *chartv
if notesTxt != "" { if notesTxt != "" {
upgradedRelease.Info.Notes = notesTxt upgradedRelease.Info.Notes = notesTxt
} }
err = validateManifest(u.cfg.KubeClient, manifestDoc.Bytes(), !u.DisableOpenAPIValidation) err = validateManifest(u.cfg.KubeClient, manifest, !u.DisableOpenAPIValidation)
return currentRelease, upgradedRelease, serverSideApply, err return currentRelease, upgradedRelease, serverSideApply, err
} }

Write Preview
Loading…
Cancel
Save