From 79784204ea09c79e6c4405e073cacee921b8670a Mon Sep 17 00:00:00 2001 From: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Date: Sun, 20 Sep 2026 15:41:31 -0700 Subject: [PATCH 1/2] fix(action): wait for CRD establishment under HookOnlyStrategy Motivation: installCRDs() selected its CRD-establishment waiter using the release's WaitStrategy. Under kube.HookOnlyStrategy, that waiter's Wait() is a no-op by design, intended to skip waiting on general chart resources such as Deployments. That no-op also silently skipped the unrelated CRD establishment check, which exists so custom resources depending on freshly-installed CRDs aren't processed before the API server recognizes the new CRD types. Helm 3 performed this CRD establishment wait unconditionally, regardless of the release's wait flag/strategy. Under HookOnlyStrategy this can intermittently produce an install failure such as: no matches for kind "ServiceMonitor" in version "monitoring.coreos.com/v1" when a chart installs CRDs and dependent custom resources in the same release, if the CRDs have not finished being established by the time Helm processes the custom resources. Approach: In installCRDs(), compute a local crdWaitStrategy that substitutes kube.StatusWatcherStrategy for the CRD-establishment wait only when i.WaitStrategy is HookOnlyStrategy. i.WaitStrategy itself is left untouched, so every other waiter-selection call site (the general-resource wait later in Install.RunWithContext, and the equivalents in rollback.go/upgrade.go) keeps HookOnlyStrategy's existing no-op behavior for general resources. This mirrors the promotion to StatusWatcherStrategy already used elsewhere in this file for HookOnlyStrategy combined with RollbackOnFailure. Validation: - go build ./... - go test ./pkg/action/... ./pkg/kube/... - golangci-lint run ./pkg/action/... ./pkg/kube/... (0 issues) - go vet ./pkg/action/... ./pkg/kube/... (clean) - go mod tidy -diff (empty) - make build && make test-coverage PKG="./pkg/action/... ./pkg/kube/..." - Added TestInstallCRDs_HookOnlyStrategyStillWaitsForEstablishment, a targeted unit test with a fake kube.Interface that mimics the real no-op hookOnlyWaiter versus a genuine wait attempt. Verified via git stash that this test fails on the pre-fix code ("An error is expected but got nil") and passes after the fix. This was validated at the unit level with a fake Kubernetes client, not against a live cluster. Report: https://github.com/helm/helm/issues/32671 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Assisted-by: claude-sonnet-5 (via Claude Code) --- pkg/action/install.go | 15 +++++++-- pkg/action/install_test.go | 66 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index 6fc919366..48b0d8bf8 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -221,10 +221,21 @@ func (i *Install) installCRDs(crds []chart.CRD) error { if len(totalItems) > 0 { var waiter kube.Waiter var err error + // CRD establishment must always be awaited, regardless of the wait + // strategy configured for the release. HookOnlyStrategy's Wait is a + // no-op by design so that it can skip waiting for general chart + // resources, but skipping it here too lets Helm start creating + // custom resources before their CRDs are recognized by the API + // server. Use StatusWatcherStrategy for this check instead so CRDs + // are still waited for, matching Helm 3's unconditional behavior. + crdWaitStrategy := i.WaitStrategy + if crdWaitStrategy == kube.HookOnlyStrategy { + crdWaitStrategy = kube.StatusWatcherStrategy + } if c, supportsOptions := i.cfg.KubeClient.(kube.InterfaceWaitOptions); supportsOptions { - waiter, err = c.GetWaiterWithOptions(i.WaitStrategy, i.WaitOptions...) + waiter, err = c.GetWaiterWithOptions(crdWaitStrategy, i.WaitOptions...) } else { - waiter, err = i.cfg.KubeClient.GetWaiter(i.WaitStrategy) + waiter, err = i.cfg.KubeClient.GetWaiter(crdWaitStrategy) } if err != nil { return fmt.Errorf("unable to get waiter: %w", err) diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index 2d83abe27..257cb8e61 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -1182,6 +1182,72 @@ func TestInstallCRDs_WaiterError(t *testing.T) { require.Error(t, instAction.installCRDs(crdsToInstall), "wait error") } +// strategyAwareWaitKubeClient is a fake kube.Interface that records which +// WaitStrategy installCRDs() requests a waiter for, and returns a waiter +// that mimics the real hookOnlyWaiter: its Wait() is a no-op under +// HookOnlyStrategy and reports the CRD as not yet established otherwise. +type strategyAwareWaitKubeClient struct { + kubefake.PrintingKubeClient + requestedStrategy kube.WaitStrategy +} + +func (k *strategyAwareWaitKubeClient) Build(_ io.Reader, _ bool) (kube.ResourceList, error) { + var resInfo resource.Info + resInfo.Name = "dummyName" + resInfo.Namespace = "dummyNamespace" + var resourceList kube.ResourceList + resourceList.Append(&resInfo) + return resourceList, nil +} + +func (k *strategyAwareWaitKubeClient) GetWaiter(ws kube.WaitStrategy) (kube.Waiter, error) { + return k.GetWaiterWithOptions(ws) +} + +func (k *strategyAwareWaitKubeClient) GetWaiterWithOptions(ws kube.WaitStrategy, _ ...kube.WaitOption) (kube.Waiter, error) { + k.requestedStrategy = ws + return &strategyAwareWaiter{strategy: ws}, nil +} + +type strategyAwareWaiter struct { + kubefake.PrintingKubeWaiter + strategy kube.WaitStrategy +} + +func (w *strategyAwareWaiter) Wait(_ kube.ResourceList, _ time.Duration) error { + if w.strategy == kube.HookOnlyStrategy { + return nil + } + return errors.New("CRD not yet established") +} + +// TestInstallCRDs_HookOnlyStrategyStillWaitsForEstablishment guards against +// the regression reported in https://github.com/helm/helm/issues/32671: with +// WaitStrategy set to HookOnlyStrategy, installCRDs() must still wait for +// CRD establishment instead of relying on HookOnlyStrategy's waiter, whose +// Wait() no-ops for general chart resources by design. Before the fix, this +// test failed because installCRDs() requested a HookOnlyStrategy waiter +// (whose no-op Wait() returned nil), swallowing the wait entirely. +func TestInstallCRDs_HookOnlyStrategyStillWaitsForEstablishment(t *testing.T) { + config := actionConfigFixture(t) + fakeClient := &strategyAwareWaitKubeClient{PrintingKubeClient: kubefake.PrintingKubeClient{Out: io.Discard}} + config.KubeClient = fakeClient + instAction := NewInstall(config) + instAction.WaitStrategy = kube.HookOnlyStrategy + + mockFile := common.File{ + Name: "crds/foo.yaml", + Data: []byte("hello"), + } + mockChart := buildChart(withFile(mockFile)) + crdsToInstall := mockChart.CRDObjects() + + err := instAction.installCRDs(crdsToInstall) + require.Error(t, err, "installCRDs should still wait for CRD establishment under HookOnlyStrategy") + assert.Contains(t, err.Error(), "CRD not yet established") + assert.Equal(t, kube.StatusWatcherStrategy, fakeClient.requestedStrategy, "CRD establishment wait must not use HookOnlyStrategy's no-op waiter") +} + func TestCheckDependencies(t *testing.T) { dependency := chart.Dependency{Name: "hello"} mockChart := buildChart(withDependency()) From ac4e9e937cdf56534048788368834917c277a640 Mon Sep 17 00:00:00 2001 From: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Date: Wed, 30 Sep 2026 23:12:15 -0700 Subject: [PATCH 2/2] test(action): reuse FailingKubeClient for the CRD wait strategy test Drop the test-only strategyAwareWaitKubeClient/strategyAwareWaiter fakes and use kubefake.FailingKubeClient instead. FailingKubeClient now records the WaitStrategy passed to GetWaiter/GetWaiterWithOptions in RecordedWaitStrategies, alongside the existing RecordedWaitOptions, so the test can assert installCRDs() asks for a StatusWatcherStrategy waiter under HookOnlyStrategy. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Assisted-by: claude-opus-5-5 (via Claude Code) --- pkg/action/install_test.go | 58 ++++------------------------ pkg/kube/fake/failing_kube_client.go | 9 +++-- 2 files changed, 13 insertions(+), 54 deletions(-) diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index 257cb8e61..f323d047e 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -1182,56 +1182,14 @@ func TestInstallCRDs_WaiterError(t *testing.T) { require.Error(t, instAction.installCRDs(crdsToInstall), "wait error") } -// strategyAwareWaitKubeClient is a fake kube.Interface that records which -// WaitStrategy installCRDs() requests a waiter for, and returns a waiter -// that mimics the real hookOnlyWaiter: its Wait() is a no-op under -// HookOnlyStrategy and reports the CRD as not yet established otherwise. -type strategyAwareWaitKubeClient struct { - kubefake.PrintingKubeClient - requestedStrategy kube.WaitStrategy -} - -func (k *strategyAwareWaitKubeClient) Build(_ io.Reader, _ bool) (kube.ResourceList, error) { - var resInfo resource.Info - resInfo.Name = "dummyName" - resInfo.Namespace = "dummyNamespace" - var resourceList kube.ResourceList - resourceList.Append(&resInfo) - return resourceList, nil -} - -func (k *strategyAwareWaitKubeClient) GetWaiter(ws kube.WaitStrategy) (kube.Waiter, error) { - return k.GetWaiterWithOptions(ws) -} - -func (k *strategyAwareWaitKubeClient) GetWaiterWithOptions(ws kube.WaitStrategy, _ ...kube.WaitOption) (kube.Waiter, error) { - k.requestedStrategy = ws - return &strategyAwareWaiter{strategy: ws}, nil -} - -type strategyAwareWaiter struct { - kubefake.PrintingKubeWaiter - strategy kube.WaitStrategy -} - -func (w *strategyAwareWaiter) Wait(_ kube.ResourceList, _ time.Duration) error { - if w.strategy == kube.HookOnlyStrategy { - return nil - } - return errors.New("CRD not yet established") -} - // TestInstallCRDs_HookOnlyStrategyStillWaitsForEstablishment guards against -// the regression reported in https://github.com/helm/helm/issues/32671: with -// WaitStrategy set to HookOnlyStrategy, installCRDs() must still wait for -// CRD establishment instead of relying on HookOnlyStrategy's waiter, whose -// Wait() no-ops for general chart resources by design. Before the fix, this -// test failed because installCRDs() requested a HookOnlyStrategy waiter -// (whose no-op Wait() returned nil), swallowing the wait entirely. +// https://github.com/helm/helm/issues/32671: HookOnlyStrategy's waiter does not +// wait on general resources, so installCRDs() must request a +// StatusWatcherStrategy waiter for CRD establishment instead. func TestInstallCRDs_HookOnlyStrategyStillWaitsForEstablishment(t *testing.T) { config := actionConfigFixture(t) - fakeClient := &strategyAwareWaitKubeClient{PrintingKubeClient: kubefake.PrintingKubeClient{Out: io.Discard}} - config.KubeClient = fakeClient + failingKubeClient := kubefake.FailingKubeClient{PrintingKubeClient: kubefake.PrintingKubeClient{Out: io.Discard}, BuildDummy: true} + config.KubeClient = &failingKubeClient instAction := NewInstall(config) instAction.WaitStrategy = kube.HookOnlyStrategy @@ -1242,10 +1200,8 @@ func TestInstallCRDs_HookOnlyStrategyStillWaitsForEstablishment(t *testing.T) { mockChart := buildChart(withFile(mockFile)) crdsToInstall := mockChart.CRDObjects() - err := instAction.installCRDs(crdsToInstall) - require.Error(t, err, "installCRDs should still wait for CRD establishment under HookOnlyStrategy") - assert.Contains(t, err.Error(), "CRD not yet established") - assert.Equal(t, kube.StatusWatcherStrategy, fakeClient.requestedStrategy, "CRD establishment wait must not use HookOnlyStrategy's no-op waiter") + require.NoError(t, instAction.installCRDs(crdsToInstall)) + assert.Equal(t, []kube.WaitStrategy{kube.StatusWatcherStrategy}, failingKubeClient.RecordedWaitStrategies) } func TestCheckDependencies(t *testing.T) { diff --git a/pkg/kube/fake/failing_kube_client.go b/pkg/kube/fake/failing_kube_client.go index 75d0c8de1..51fab26af 100644 --- a/pkg/kube/fake/failing_kube_client.go +++ b/pkg/kube/fake/failing_kube_client.go @@ -50,7 +50,9 @@ type FailingKubeClient struct { WaitDuration time.Duration // RecordedWaitOptions stores the WaitOptions passed to GetWaiter for testing RecordedWaitOptions []kube.WaitOption - mu sync.Mutex + // RecordedWaitStrategies stores the WaitStrategy passed to each GetWaiter call for testing + RecordedWaitStrategies []kube.WaitStrategy + mu sync.Mutex } var _ kube.Interface = &FailingKubeClient{} @@ -160,14 +162,15 @@ func (f *FailingKubeClient) GetWaiter(ws kube.WaitStrategy) (kube.Waiter, error) return f.GetWaiterWithOptions(ws) } -func (f *FailingKubeClient) appendRecordedWaitOptionsLocked(opts ...kube.WaitOption) { +func (f *FailingKubeClient) recordGetWaiterCallLocked(ws kube.WaitStrategy, opts ...kube.WaitOption) { f.mu.Lock() defer f.mu.Unlock() + f.RecordedWaitStrategies = append(f.RecordedWaitStrategies, ws) f.RecordedWaitOptions = append(f.RecordedWaitOptions, opts...) } func (f *FailingKubeClient) GetWaiterWithOptions(ws kube.WaitStrategy, opts ...kube.WaitOption) (kube.Waiter, error) { - f.appendRecordedWaitOptionsLocked(opts...) + f.recordGetWaiterCallLocked(ws, opts...) waiter, _ := f.PrintingKubeClient.GetWaiterWithOptions(ws, opts...) printingKubeWaiter, _ := waiter.(*PrintingKubeWaiter) return &FailingKubeWaiter{