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] 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())