upstream/helm
1
0
Fork 0
mirror of https://github.com/helm/helm.git synced 2026-05-28 04:35:48 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Merge branch 'helm:main' into templates_lint

Browse source
This commit is contained in:
Zach Burgess 2025-07-07 09:34:04 -07:00 committed by GitHub
commit 6e30619d8f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 350 additions and 75 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
pkg/action/rollback.go
View file

@ -41,7 +41,6 @@ type Rollback struct {
WaitForJobs bool
DisableHooks bool
DryRun bool
Recreate bool // will (if true) recreate pods after a rollback.
Force bool // will (if true) force resource upgrade through uninstall/recreate if needed
CleanupOnFail bool
MaxHistory int // MaxHistory limits the maximum number of revisions saved per release
@ -211,15 +210,6 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas
return targetRelease, err
}
if r.Recreate {
// NOTE: Because this is not critical for a release to succeed, we just
// log if an error occurs and continue onward. If we ever introduce log
// levels, we should make these error level logs so users are notified
// that they'll need to go do the cleanup on their own
if err := recreate(r.cfg, results.Updated); err != nil {
slog.Error(err.Error())
}
}
waiter, err := r.cfg.KubeClient.GetWaiter(r.WaitStrategy)
if err != nil {
return nil, fmt.Errorf("unable to set metadata visitor from target release: %w", err)

49
pkg/action/upgrade.go
View file

@ -26,7 +26,6 @@ import (
"sync"
"time"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/cli-runtime/pkg/resource"
chart "helm.sh/helm/v4/pkg/chart/v2"
@ -88,8 +87,6 @@ type Upgrade struct {
ReuseValues bool
// ResetThenReuseValues will reset the values to the chart's built-ins then merge with user's last supplied values.
ResetThenReuseValues bool
// Recreate will (if true) recreate pods after a rollback.
Recreate bool
// MaxHistory limits the maximum number of revisions saved per release
MaxHistory int
// Atomic, if true, will roll back on failure.
@ -436,15 +433,6 @@ func (u *Upgrade) releasingUpgrade(c chan<- resultMessage, upgradedRelease *rele
return
}
if u.Recreate {
// NOTE: Because this is not critical for a release to succeed, we just
// log if an error occurs and continue onward. If we ever introduce log
// levels, we should make these error level logs so users are notified
// that they'll need to go do the cleanup on their own
if err := recreate(u.cfg, results.Updated); err != nil {
slog.Error(err.Error())
}
}
waiter, err := u.cfg.KubeClient.GetWaiter(u.WaitStrategy)
if err != nil {
u.cfg.recordRelease(originalRelease)
@ -537,7 +525,6 @@ func (u *Upgrade) failRelease(rel *release.Release, created kube.ResourceList, e
}
rollin.WaitForJobs = u.WaitForJobs
rollin.DisableHooks = u.DisableHooks
rollin.Recreate = u.Recreate
rollin.Force = u.Force
rollin.Timeout = u.Timeout
if rollErr := rollin.Run(rel.Name); rollErr != nil {
@ -602,42 +589,6 @@ func validateManifest(c kube.Interface, manifest []byte, openAPIValidation bool)
return err
}
// recreate captures all the logic for recreating pods for both upgrade and
// rollback. If we end up refactoring rollback to use upgrade, this can just be
// made an unexported method on the upgrade action.
func recreate(cfg *Configuration, resources kube.ResourceList) error {
for _, res := range resources {
versioned := kube.AsVersioned(res)
selector, err := kube.SelectorsForObject(versioned)
if err != nil {
// If no selector is returned, it means this object is
// definitely not a pod, so continue onward
continue
}
client, err := cfg.KubernetesClientSet()
if err != nil {
return fmt.Errorf("unable to recreate pods for object %s/%s because an error occurred: %w", res.Namespace, res.Name, err)
}
pods, err := client.CoreV1().Pods(res.Namespace).List(context.Background(), metav1.ListOptions{
LabelSelector: selector.String(),
})
if err != nil {
return fmt.Errorf("unable to recreate pods for object %s/%s because an error occurred: %w", res.Namespace, res.Name, err)
}
// Restart pods
for _, pod := range pods.Items {
// Delete each pod for get them restarted with changed spec.
if err := client.CoreV1().Pods(pod.Namespace).Delete(context.Background(), pod.Name, *metav1.NewPreconditionDeleteOptions(string(pod.UID))); err != nil {
return fmt.Errorf("unable to recreate pods for object %s/%s because an error occurred: %w", res.Namespace, res.Name, err)
}
}
}
return nil
}
func objectKey(r *resource.Info) string {
gvk := r.Object.GetObjectKind().GroupVersionKind()
return fmt.Sprintf("%s/%s/%s/%s", gvk.GroupVersion().String(), gvk.Kind, r.Namespace, r.Name)

11
pkg/cmd/create_test.go
View file

@ -30,10 +30,9 @@ import (
)
func TestCreateCmd(t *testing.T) {
t.Chdir(t.TempDir())
ensure.HelmHome(t)
cname := "testchart"
dir := t.TempDir()
defer t.Chdir(dir)
// Run a create
if _, _, err := executeActionCommand("create " + cname); err != nil {
@ -61,12 +60,10 @@ func TestCreateCmd(t *testing.T) {
}
func TestCreateStarterCmd(t *testing.T) {
t.Chdir(t.TempDir())
ensure.HelmHome(t)
cname := "testchart"
defer resetEnv()()
os.MkdirAll(helmpath.CachePath(), 0o755)
defer t.Chdir(helmpath.CachePath())
// Create a starter.
starterchart := helmpath.DataPath("starters")
os.MkdirAll(starterchart, 0o755)
@ -125,6 +122,7 @@ func TestCreateStarterCmd(t *testing.T) {
}
func TestCreateStarterAbsoluteCmd(t *testing.T) {
t.Chdir(t.TempDir())
defer resetEnv()()
ensure.HelmHome(t)
cname := "testchart"
@ -142,9 +140,6 @@ func TestCreateStarterAbsoluteCmd(t *testing.T) {
t.Fatalf("Could not write template: %s", err)
}
os.MkdirAll(helmpath.CachePath(), 0o755)
defer t.Chdir(helmpath.CachePath())
starterChartPath := filepath.Join(starterchart, "starterchart")
// Run a create

5
pkg/cmd/package_test.go
View file

@ -23,6 +23,7 @@ import (
"strings"
"testing"
"helm.sh/helm/v4/internal/test/ensure"
chart "helm.sh/helm/v4/pkg/chart/v2"
"helm.sh/helm/v4/pkg/chart/v2/loader"
)
@ -110,8 +111,8 @@ func TestPackage(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cachePath := t.TempDir()
defer t.Chdir(cachePath)
t.Chdir(t.TempDir())
ensure.HelmHome(t)
if err := os.MkdirAll("toot", 0o777); err != nil {
t.Fatal(err)

1
pkg/cmd/rollback.go
View file

@ -77,7 +77,6 @@ func newRollbackCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
f := cmd.Flags()
f.BoolVar(&client.DryRun, "dry-run", false, "simulate a rollback")
f.BoolVar(&client.Recreate, "recreate-pods", false, "performs pods restart for the resource if applicable")
f.BoolVar(&client.Force, "force", false, "force resource update through delete/recreate if needed")
f.BoolVar(&client.DisableHooks, "no-hooks", false, "prevent hooks from running during rollback")
f.DurationVar(&client.Timeout, "timeout", 300*time.Second, "time to wait for any individual Kubernetes operation (like Jobs for hooks)")

2
pkg/cmd/template.go
View file

@ -201,7 +201,7 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
f.BoolVar(&skipTests, "skip-tests", false, "skip tests from templated output")
f.BoolVar(&client.IsUpgrade, "is-upgrade", false, "set .Release.IsUpgrade instead of .Release.IsInstall")
f.StringVar(&kubeVersion, "kube-version", "", "Kubernetes version used for Capabilities.KubeVersion")
f.StringSliceVarP(&extraAPIs, "api-versions", "a", []string{}, "Kubernetes api versions used for Capabilities.APIVersions")
f.StringSliceVarP(&extraAPIs, "api-versions", "a", []string{}, "Kubernetes api versions used for Capabilities.APIVersions (multiple can be specified)")
f.BoolVar(&client.UseReleaseName, "release-name", false, "use release name in the output-dir path.")
bindPostRenderFlag(cmd, &client.PostRenderer)

7
pkg/cmd/template_test.go
View file

@ -83,7 +83,12 @@ func TestTemplateCmd(t *testing.T) {
},
{
name: "check kube api versions",
cmd: fmt.Sprintf("template --api-versions helm.k8s.io/test '%s'", chartPath),
cmd: fmt.Sprintf("template --api-versions helm.k8s.io/test,helm.k8s.io/test2 '%s'", chartPath),
golden: "output/template-with-api-version.txt",
},
{
name: "check kube api versions",
cmd: fmt.Sprintf("template --api-versions helm.k8s.io/test --api-versions helm.k8s.io/test2 '%s'", chartPath),
golden: "output/template-with-api-version.txt",
},
{

1
pkg/cmd/testdata/output/template-with-api-version.txt vendored
View file

@ -75,6 +75,7 @@ metadata:
kube-version/minor: "20"
kube-version/version: "v1.20.0"
kube-api-version/test: v1
kube-api-version/test2: v2
spec:
type: ClusterIP
ports:

3
pkg/cmd/testdata/testcharts/subchart/templates/service.yaml vendored
View file

@ -11,6 +11,9 @@ metadata:
{{- if .Capabilities.APIVersions.Has "helm.k8s.io/test" }}
kube-api-version/test: v1
{{- end }}
{{- if .Capabilities.APIVersions.Has "helm.k8s.io/test2" }}
kube-api-version/test2: v2
{{- end }}
spec:
type: {{ .Values.service.type }}
ports:

2
pkg/cmd/upgrade.go
View file

@ -268,8 +268,6 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
f.StringVar(&client.DryRunOption, "dry-run", "", "simulate an install. If --dry-run is set with no option being specified or as '--dry-run=client', it will not attempt cluster connections. Setting '--dry-run=server' allows attempting cluster connections.")
f.BoolVar(&client.HideSecret, "hide-secret", false, "hide Kubernetes Secrets when also using the --dry-run flag")
f.Lookup("dry-run").NoOptDefVal = "client"
f.BoolVar(&client.Recreate, "recreate-pods", false, "performs pods restart for the resource if applicable")
f.MarkDeprecated("recreate-pods", "functionality will no longer be updated. Consult the documentation for other methods to recreate pods")
f.BoolVar(&client.Force, "force", false, "force resource updates through a replacement strategy")
f.BoolVar(&client.DisableHooks, "no-hooks", false, "disable pre/post upgrade hooks")
f.BoolVar(&client.DisableOpenAPIValidation, "disable-openapi-validation", false, "if set, the upgrade process will not validate rendered templates against the Kubernetes OpenAPI Schema")

2
pkg/lint/support/message_test.go
View file

@ -21,7 +21,6 @@ import (
"testing"
)
var linter = Linter{}
var errLint = errors.New("lint failed")
func TestRunLinterRule(t *testing.T) {
@ -45,6 +44,7 @@ func TestRunLinterRule(t *testing.T) {
{-1, errLint, 4, false, ErrorSev},
}
linter := Linter{}
for _, test := range tests {
isValid := linter.RunLinterRule(test.Severity, "chart", test.LintError)
if len(linter.Messages) != test.ExpectedMessages {

332
pkg/pusher/ocipusher_test.go
View file

@ -1,3 +1,5 @@
//go:build !windows
/*
Copyright The Helm Authors.
Licensed under the Apache License, Version 2.0 (the "License");
@ -16,7 +18,10 @@ limitations under the License.
package pusher
import (
"io"
"os"
"path/filepath"
"strings"
"testing"
"helm.sh/helm/v4/pkg/registry"
@ -94,3 +99,330 @@ func TestNewOCIPusher(t *testing.T) {
t.Errorf("Expected NewOCIPusher to contain %p as RegistryClient, got %p", registryClient, op.opts.registryClient)
}
}
func TestOCIPusher_Push_ErrorHandling(t *testing.T) {
tests := []struct {
name string
chartRef string
expectedError string
setupFunc func() string
}{
{
name: "non-existent file",
chartRef: "/non/existent/file.tgz",
expectedError: "no such file",
},
{
name: "directory instead of file",
expectedError: "cannot push directory, must provide chart archive (.tgz)",
setupFunc: func() string {
tempDir := t.TempDir()
return tempDir
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
pusher, err := NewOCIPusher()
if err != nil {
t.Fatal(err)
}
chartRef := tt.chartRef
if tt.setupFunc != nil {
chartRef = tt.setupFunc()
}
err = pusher.Push(chartRef, "oci://localhost:5000/test")
if err == nil {
t.Fatal("Expected error but got none")
}
if !strings.Contains(err.Error(), tt.expectedError) {
t.Errorf("Expected error containing %q, got %q", tt.expectedError, err.Error())
}
})
}
}
func TestOCIPusher_newRegistryClient(t *testing.T) {
cd := "../../testdata"
join := filepath.Join
ca, pub, priv := join(cd, "rootca.crt"), join(cd, "crt.pem"), join(cd, "key.pem")
tests := []struct {
name string
opts []Option
expectError bool
errorContains string
}{
{
name: "plain HTTP",
opts: []Option{WithPlainHTTP(true)},
},
{
name: "with TLS client config",
opts: []Option{
WithTLSClientConfig(pub, priv, ca),
},
},
{
name: "with insecure skip TLS verify",
opts: []Option{
WithInsecureSkipTLSVerify(true),
},
},
{
name: "with cert and key only",
opts: []Option{
WithTLSClientConfig(pub, priv, ""),
},
},
{
name: "with CA file only",
opts: []Option{
WithTLSClientConfig("", "", ca),
},
},
{
name: "default client without options",
opts: []Option{},
},
{
name: "invalid cert file",
opts: []Option{
WithTLSClientConfig("/non/existent/cert.pem", priv, ca),
},
expectError: true,
errorContains: "can't create TLS config",
},
{
name: "invalid key file",
opts: []Option{
WithTLSClientConfig(pub, "/non/existent/key.pem", ca),
},
expectError: true,
errorContains: "can't create TLS config",
},
{
name: "invalid CA file",
opts: []Option{
WithTLSClientConfig("", "", "/non/existent/ca.crt"),
},
expectError: true,
errorContains: "can't create TLS config",
},
{
name: "combined TLS options",
opts: []Option{
WithTLSClientConfig(pub, priv, ca),
WithInsecureSkipTLSVerify(true),
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
pusher, err := NewOCIPusher(tt.opts...)
if err != nil {
t.Fatal(err)
}
op, ok := pusher.(*OCIPusher)
if !ok {
t.Fatal("Expected *OCIPusher")
}
client, err := op.newRegistryClient()
if tt.expectError {
if err == nil {
t.Fatal("Expected error but got none")
}
if tt.errorContains != "" && !strings.Contains(err.Error(), tt.errorContains) {
t.Errorf("Expected error containing %q, got %q", tt.errorContains, err.Error())
}
} else {
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
if client == nil {
t.Fatal("Expected non-nil registry client")
}
}
})
}
}
func TestOCIPusher_Push_ChartOperations(t *testing.T) {
// Path to test charts
chartPath := "../../pkg/cmd/testdata/testcharts/compressedchart-0.1.0.tgz"
chartWithProvPath := "../../pkg/cmd/testdata/testcharts/signtest-0.1.0.tgz"
tests := []struct {
name string
chartRef string
href string
options []Option
setupFunc func(t *testing.T) (string, func())
expectError bool
errorContains string
}{
{
name: "invalid chart file",
chartRef: "../../pkg/action/testdata/charts/corrupted-compressed-chart.tgz",
href: "oci://localhost:5000/test",
expectError: true,
errorContains: "does not appear to be a gzipped archive",
},
{
name: "chart read error",
setupFunc: func(t *testing.T) (string, func()) {
t.Helper()
// Create a valid chart file that we'll make unreadable
tempDir := t.TempDir()
tempChart := filepath.Join(tempDir, "temp-chart.tgz")
// Copy a valid chart
src, err := os.Open(chartPath)
if err != nil {
t.Fatal(err)
}
defer src.Close()
dst, err := os.Create(tempChart)
if err != nil {
t.Fatal(err)
}
if _, err := io.Copy(dst, src); err != nil {
t.Fatal(err)
}
dst.Close()
// Make the file unreadable
if err := os.Chmod(tempChart, 0000); err != nil {
t.Fatal(err)
}
return tempChart, func() {
os.Chmod(tempChart, 0644) // Restore permissions for cleanup
}
},
href: "oci://localhost:5000/test",
expectError: true,
errorContains: "permission denied",
},
{
name: "push with provenance file - loading phase",
chartRef: chartWithProvPath,
href: "oci://registry.example.com/charts",
setupFunc: func(t *testing.T) (string, func()) {
t.Helper()
// Copy chart and create a .prov file for it
tempDir := t.TempDir()
tempChart := filepath.Join(tempDir, "signtest-0.1.0.tgz")
tempProv := filepath.Join(tempDir, "signtest-0.1.0.tgz.prov")
// Copy chart file
src, err := os.Open(chartWithProvPath)
if err != nil {
t.Fatal(err)
}
defer src.Close()
dst, err := os.Create(tempChart)
if err != nil {
t.Fatal(err)
}
if _, err := io.Copy(dst, src); err != nil {
t.Fatal(err)
}
dst.Close()
// Create provenance file
if err := os.WriteFile(tempProv, []byte("test provenance data"), 0644); err != nil {
t.Fatal(err)
}
return tempChart, func() {}
},
expectError: true, // Will fail at the registry push step
errorContains: "", // Error depends on registry client behavior
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
chartRef := tt.chartRef
var cleanup func()
if tt.setupFunc != nil {
chartRef, cleanup = tt.setupFunc(t)
if cleanup != nil {
defer cleanup()
}
}
// Skip test if chart file doesn't exist and we're not expecting an error
if _, err := os.Stat(chartRef); err != nil && !tt.expectError {
t.Skipf("Test chart %s not found, skipping test", chartRef)
}
pusher, err := NewOCIPusher(tt.options...)
if err != nil {
t.Fatal(err)
}
err = pusher.Push(chartRef, tt.href)
if tt.expectError {
if err == nil {
t.Fatal("Expected error but got none")
}
if tt.errorContains != "" && !strings.Contains(err.Error(), tt.errorContains) {
t.Errorf("Expected error containing %q, got %q", tt.errorContains, err.Error())
}
} else {
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
}
})
}
}
func TestOCIPusher_Push_MultipleOptions(t *testing.T) {
chartPath := "../../pkg/cmd/testdata/testcharts/compressedchart-0.1.0.tgz"
// Skip test if chart file doesn't exist
if _, err := os.Stat(chartPath); err != nil {
t.Skipf("Test chart %s not found, skipping test", chartPath)
}
pusher, err := NewOCIPusher()
if err != nil {
t.Fatal(err)
}
// Test that multiple options are applied correctly
err = pusher.Push(chartPath, "oci://localhost:5000/test",
WithPlainHTTP(true),
WithInsecureSkipTLSVerify(true),
)
// We expect an error since we're not actually pushing to a registry
if err == nil {
t.Fatal("Expected error when pushing without a valid registry")
}
// Verify options were applied
op := pusher.(*OCIPusher)
if !op.opts.plainHTTP {
t.Error("Expected plainHTTP option to be applied")
}
if !op.opts.insecureSkipTLSverify {
t.Error("Expected insecureSkipTLSverify option to be applied")
}
}