From e1331e031d21d55708dd1b0689201636634dd612 Mon Sep 17 00:00:00 2001 From: Raphael Fakhri <153192858+RaphaelFakhri@users.noreply.github.com> Date: Wed, 4 Nov 2026 07:44:14 +0000 Subject: [PATCH 1/2] pkg/storage: fix goroutine leak when chunked release decoding fails decodeRelease streams chunk data to the gzip and JSON decoders through an io.Pipe, but never closes the read side. If decoding fails before all of the chunk data is read, for example because of a corrupt or truncated chunk, the writer goroutine blocks on pw.Write forever and keeps the secret data in memory. Each failed Get, List or Query leaks one goroutine, so an operator that repeatedly reconciles such a release grows without bound. Close the pipe reader when decodeRelease returns so the pending write fails and the goroutine exits. --- pkg/storage/chunked.go | 3 +++ pkg/storage/chunked_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/pkg/storage/chunked.go b/pkg/storage/chunked.go index be411e66..29036d97 100644 --- a/pkg/storage/chunked.go +++ b/pkg/storage/chunked.go @@ -398,6 +398,9 @@ func (c *chunkedSecrets) decodeRelease(ctx context.Context, indexSecret *corev1. } pr, pw := io.Pipe() + // Close the read side when decoding returns so that the writer goroutine + // does not block forever if decoding stops before all chunk data is read. + defer pr.Close() go func() { defer pw.Close() firstChunkData, ok := indexSecret.Data["chunk"] diff --git a/pkg/storage/chunked_test.go b/pkg/storage/chunked_test.go index d8bd0c12..3d9fba91 100644 --- a/pkg/storage/chunked_test.go +++ b/pkg/storage/chunked_test.go @@ -5,6 +5,8 @@ import ( "encoding/json" "fmt" "maps" + "runtime" + "strings" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -95,6 +97,26 @@ var _ = Describe("chunkedSecrets", func() { _, err := maxReadDriver.Get(releaseKey(rel)) Expect(err).To(MatchError(ContainSubstring("release too large"))) }) + It("should not leak goroutines if the release fails to decode", func() { + corruptIndex := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "corrupt-release.v1"}, + Type: SecretTypeChunkedIndex, + Data: map[string][]byte{ + "extraChunks": []byte("[]"), + // Larger than the gzip reader's read buffer, so the + // reader stops before consuming all of the chunk data. + "chunk": make([]byte, 10*chunkSize), + }, + } + _, err := secretInterface.Create(context.Background(), corruptIndex, metav1.CreateOptions{}) + Expect(err).ToNot(HaveOccurred()) + + for range 5 { + _, err := chunkedDriver.Get(corruptIndex.Name) + Expect(err).To(MatchError(ContainSubstring("failed to create gzip reader"))) + } + Eventually(countDecodeReleaseGoroutines).Should(BeZero()) + }) }) var _ = Describe("Update", func() { @@ -327,6 +349,17 @@ func verifySecrets(secretInterface clientcorev1.SecretInterface, expected int) { Expect(actualExtraChunkNames).To(ConsistOf(expectedExtraChunkNames)) } +func countDecodeReleaseGoroutines() int { + buf := make([]byte, 1<<20) + for { + n := runtime.Stack(buf, true) + if n < len(buf) { + return strings.Count(string(buf[:n]), "(*chunkedSecrets).decodeRelease.func") + } + buf = make([]byte, 2*len(buf)) + } +} + func releaseKey(rel *release.Release) string { return fmt.Sprintf("%s.v%d", rel.Name, rel.Version) } From 7d52c3329a76efd4eea5ed6e6669df5c1ce4d97c Mon Sep 17 00:00:00 2001 From: Raphael Fakhri <153192858+RaphaelFakhri@users.noreply.github.com> Date: Fri, 6 Nov 2026 20:41:06 +0000 Subject: [PATCH 2/2] pkg/storage: check the goroutine count instead of stack traces Count all goroutines with runtime.NumGoroutine, so the leak test does not depend on the stack trace format or on function names. Move the deferred pr.Close next to the gzip reader that consumes pr. --- pkg/storage/chunked.go | 6 +++--- pkg/storage/chunked_test.go | 15 ++------------- 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/pkg/storage/chunked.go b/pkg/storage/chunked.go index 29036d97..2dcff5dc 100644 --- a/pkg/storage/chunked.go +++ b/pkg/storage/chunked.go @@ -398,9 +398,6 @@ func (c *chunkedSecrets) decodeRelease(ctx context.Context, indexSecret *corev1. } pr, pw := io.Pipe() - // Close the read side when decoding returns so that the writer goroutine - // does not block forever if decoding stops before all chunk data is read. - defer pr.Close() go func() { defer pw.Close() firstChunkData, ok := indexSecret.Data["chunk"] @@ -430,6 +427,9 @@ func (c *chunkedSecrets) decodeRelease(ctx context.Context, indexSecret *corev1. } }() + // Close the read side when decoding returns so that the writer goroutine + // does not block forever if decoding stops before all chunk data is read. + defer pr.Close() gzr, err := gzip.NewReader(pr) if err != nil { return nil, fmt.Errorf("failed to create gzip reader: %w", err) diff --git a/pkg/storage/chunked_test.go b/pkg/storage/chunked_test.go index 3d9fba91..e99901c6 100644 --- a/pkg/storage/chunked_test.go +++ b/pkg/storage/chunked_test.go @@ -6,7 +6,6 @@ import ( "fmt" "maps" "runtime" - "strings" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -111,11 +110,12 @@ var _ = Describe("chunkedSecrets", func() { _, err := secretInterface.Create(context.Background(), corruptIndex, metav1.CreateOptions{}) Expect(err).ToNot(HaveOccurred()) + goroutinesBefore := runtime.NumGoroutine() for range 5 { _, err := chunkedDriver.Get(corruptIndex.Name) Expect(err).To(MatchError(ContainSubstring("failed to create gzip reader"))) } - Eventually(countDecodeReleaseGoroutines).Should(BeZero()) + Eventually(runtime.NumGoroutine).Should(BeNumerically("<=", goroutinesBefore)) }) }) @@ -349,17 +349,6 @@ func verifySecrets(secretInterface clientcorev1.SecretInterface, expected int) { Expect(actualExtraChunkNames).To(ConsistOf(expectedExtraChunkNames)) } -func countDecodeReleaseGoroutines() int { - buf := make([]byte, 1<<20) - for { - n := runtime.Stack(buf, true) - if n < len(buf) { - return strings.Count(string(buf[:n]), "(*chunkedSecrets).decodeRelease.func") - } - buf = make([]byte, 2*len(buf)) - } -} - func releaseKey(rel *release.Release) string { return fmt.Sprintf("%s.v%d", rel.Name, rel.Version) }