Skip to content

Commit 03de71e

Browse files
committed
fix: reduce chunked storage ChunkSize to fit within Kubernetes Secret limit
OCPBUGS-85508 The chunked Helm release storage driver set ChunkSize to exactly 1MB (1,048,576 bytes), which is the Kubernetes Secret data limit. The index Secret stores both the first chunk and a JSON list of extra chunk names (extraChunks), so any release requiring chunking exceeded the limit: Secret "sh.helm.release.v1.odf-operator.v1" is invalid: data: Too long: may not be more than 1048576 bytes Reduce ChunkSize to (1024-8)*1024 (1,040,384 bytes), leaving 8KB of headroom for the extraChunks field, and increase MaxReadChunks and MaxWriteChunks from 10 to 11 to maintain total capacity above the previous theoretical maximum.
1 parent 869124a commit 03de71e

3 files changed

Lines changed: 193 additions & 6 deletions

File tree

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
package action
2+
3+
import (
4+
"log"
5+
"os"
6+
"testing"
7+
"time"
8+
9+
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
10+
"k8s.io/client-go/rest"
11+
12+
"github.com/operator-framework/operator-controller/test"
13+
)
14+
15+
var cfg *rest.Config
16+
17+
func TestMain(m *testing.M) {
18+
testEnv := test.NewEnv()
19+
20+
var err error
21+
cfg, err = testEnv.Start()
22+
utilruntime.Must(err)
23+
if cfg == nil {
24+
log.Panic("expected cfg to not be nil")
25+
}
26+
27+
code := m.Run()
28+
stopErr := test.StopWithRetry(testEnv, time.Minute, time.Second)
29+
utilruntime.Must(stopErr)
30+
os.Exit(code)
31+
}

internal/operator-controller/action/storagedriver.go

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,24 @@ import (
1919
"github.com/operator-framework/helm-operator-plugins/pkg/storage"
2020
)
2121

22+
// chunkedSecretsConfig defines the chunked storage configuration for Helm release secrets.
23+
//
24+
// Kubernetes limits the total size of a Secret's data values to 1MB (1,048,576
25+
// bytes). The index Secret stores two data keys: "chunk" (up to ChunkSize bytes)
26+
// and "extraChunks" (a JSON array of extra chunk Secret names). ChunkSize is set
27+
// 8KB below the 1MB limit to leave headroom for the extraChunks field, whose
28+
// worst-case size is ~2.6KB (10 entries at the 253-char DNS subdomain maximum).
29+
//
30+
// MaxWriteChunks is set to 11 so that total capacity (ChunkSize * MaxWriteChunks)
31+
// exceeds the previous configuration's theoretical maximum (1MB * 10 = 10MB).
32+
// These values are pinned by tests in storagedriver_test.go and must never
33+
// decrease, as doing so would cause previously-storable releases to fail.
34+
var chunkedSecretsConfig = storage.ChunkedSecretsConfig{
35+
ChunkSize: (1024 - 8) * 1024,
36+
MaxReadChunks: 11,
37+
MaxWriteChunks: 11,
38+
}
39+
2240
func ChunkedStorageDriverMapper(secretsGetter clientcorev1.SecretsGetter, reader client.Reader, namespace string) helmclient.ObjectToStorageDriverMapper {
2341
secretsClient := newSecretsDelegatingClient(secretsGetter, reader, namespace)
2442
return func(ctx context.Context, object client.Object, config *rest.Config) (driver.Driver, error) {
@@ -27,12 +45,9 @@ func ChunkedStorageDriverMapper(secretsGetter clientcorev1.SecretsGetter, reader
2745
ownerRefSecretClient := helmclient.NewOwnerRefSecretClient(secretsClient, ownerRefs, func(secret *corev1.Secret) bool {
2846
return secret.Type == storage.SecretTypeChunkedIndex
2947
})
30-
return storage.NewChunkedSecrets(ownerRefSecretClient, "operator-controller", storage.ChunkedSecretsConfig{
31-
ChunkSize: 1024 * 1024,
32-
MaxReadChunks: 10,
33-
MaxWriteChunks: 10,
34-
Log: func(format string, args ...interface{}) { log.Info(fmt.Sprintf(format, args...)) },
35-
}), nil
48+
cfg := chunkedSecretsConfig
49+
cfg.Log = func(format string, args ...interface{}) { log.Info(fmt.Sprintf(format, args...)) }
50+
return storage.NewChunkedSecrets(ownerRefSecretClient, "operator-controller", cfg), nil
3651
}
3752
}
3853

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
package action
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"fmt"
7+
"math/rand"
8+
"testing"
9+
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
"helm.sh/helm/v3/pkg/release"
13+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
14+
clientcorev1 "k8s.io/client-go/kubernetes/typed/core/v1"
15+
16+
"github.com/operator-framework/helm-operator-plugins/pkg/storage"
17+
)
18+
19+
const (
20+
// maxCompressedReleaseSize is the maximum total size in bytes of a
21+
// gzip-compressed Helm release that the chunked storage driver can store
22+
// (ChunkSize * MaxWriteChunks). It is tested for exact equality with the
23+
// config so that any config increase forces this const to be raised too.
24+
// This value must never decrease: lowering it would cause previously-storable
25+
// releases to fail.
26+
maxCompressedReleaseSize = (1024 - 8) * 1024 * 11 // 1016 KB * 11 chunks = 11,176 KB
27+
28+
// minMaxWriteChunks is tested for exact equality with MaxWriteChunks so
29+
// that any config increase forces this const to be raised too. This value
30+
// must never decrease: lowering it would cause previously-storable releases
31+
// to fail.
32+
minMaxWriteChunks = 11
33+
34+
// minMaxReadChunks is tested for exact equality with MaxReadChunks so
35+
// that any config increase forces this const to be raised too. This value
36+
// must never decrease: lowering it would make previously-stored releases
37+
// unreadable.
38+
minMaxReadChunks = 11
39+
)
40+
41+
func TestChunkedSecretsConfigTotalCapacity(t *testing.T) {
42+
assert.Equal(t, maxCompressedReleaseSize, chunkedSecretsConfig.ChunkSize*chunkedSecretsConfig.MaxWriteChunks,
43+
"ChunkSize * MaxWriteChunks must equal maxCompressedReleaseSize")
44+
}
45+
46+
func TestChunkedSecretsConfigMaxWriteChunks(t *testing.T) {
47+
assert.Equal(t, minMaxWriteChunks, chunkedSecretsConfig.MaxWriteChunks,
48+
"MaxWriteChunks changed — update minMaxWriteChunks to match (it must never decrease)")
49+
}
50+
51+
func TestChunkedSecretsConfigMaxReadChunks(t *testing.T) {
52+
assert.Equal(t, minMaxReadChunks, chunkedSecretsConfig.MaxReadChunks,
53+
"MaxReadChunks changed — update minMaxReadChunks to match (it must never decrease)")
54+
assert.GreaterOrEqual(t, chunkedSecretsConfig.MaxReadChunks, chunkedSecretsConfig.MaxWriteChunks,
55+
"MaxReadChunks must be >= MaxWriteChunks so any written release can be read back")
56+
}
57+
58+
func TestChunkedSecretsMaxCapacityRelease(t *testing.T) {
59+
// Regression test: stores a release through the real chunked storage driver
60+
// that fills all MaxWriteChunks chunks, reads it back, and verifies the
61+
// first MaxWriteChunks-1 chunks are exactly ChunkSize bytes. This proves
62+
// the configured ChunkSize fits within the Kubernetes 1MB Secret data limit
63+
// end-to-end against a real API server.
64+
65+
chunkSize := chunkedSecretsConfig.ChunkSize
66+
maxChunks := chunkedSecretsConfig.MaxWriteChunks
67+
68+
// Use a large incompressible payload to guarantee all chunks are used.
69+
// Raw []byte in Config is base64-encoded by json.Marshal, giving full
70+
// 8-bit entropy. The base64 expansion (~33%) and gzip compression roughly
71+
// cancel out at a ~1.004 ratio, so ChunkSize*10 raw bytes compresses to
72+
// just over 10 chunks, requiring all 11.
73+
rel := &release.Release{
74+
Name: "max-capacity-test",
75+
Version: 1,
76+
Config: map[string]any{"payload": deterministicPayload(chunkSize * (maxChunks - 1))},
77+
Info: &release.Info{Status: release.StatusDeployed},
78+
}
79+
80+
secretsClient := clientcorev1.NewForConfigOrDie(cfg).Secrets("default")
81+
drv := storage.NewChunkedSecrets(secretsClient, "test-owner", chunkedSecretsConfig)
82+
83+
key := fmt.Sprintf("sh.helm.release.v1.%s.v%d", rel.Name, rel.Version)
84+
require.NoError(t, drv.Create(key, rel))
85+
86+
// Verify round-trip
87+
actual, err := drv.Get(key)
88+
require.NoError(t, err)
89+
assert.Equal(t, rel.Name, actual.Name)
90+
assert.Equal(t, rel.Version, actual.Version)
91+
92+
// Collect secrets and verify chunk sizes using the extraChunks field
93+
// from the index Secret to determine ordering.
94+
allSecrets, err := secretsClient.List(context.Background(), metav1.ListOptions{})
95+
require.NoError(t, err)
96+
97+
secretsByName := map[string][]byte{}
98+
var indexChunkData []byte
99+
var extraChunkNames []string
100+
for _, s := range allSecrets.Items {
101+
switch s.Type {
102+
case storage.SecretTypeChunkedIndex:
103+
indexChunkData = s.Data["chunk"]
104+
require.NoError(t, json.Unmarshal(s.Data["extraChunks"], &extraChunkNames))
105+
case storage.SecretTypeChunkedChunk:
106+
secretsByName[s.Name] = s.Data["chunk"]
107+
}
108+
}
109+
110+
require.NotNil(t, indexChunkData, "index Secret must exist")
111+
require.Len(t, extraChunkNames, maxChunks-1,
112+
"release must use all %d chunks", maxChunks)
113+
114+
// The first 10 chunks (index + 9 extras) must be exactly ChunkSize.
115+
// The last chunk may be smaller since it holds the remainder.
116+
assert.Equalf(t, chunkSize, len(indexChunkData),
117+
"chunk 1/%d (index) must be exactly ChunkSize", maxChunks)
118+
for i, name := range extraChunkNames[:maxChunks-2] {
119+
chunkData, ok := secretsByName[name]
120+
require.True(t, ok, "chunk Secret %q not found", name)
121+
assert.Equalf(t, chunkSize, len(chunkData),
122+
"chunk %d/%d must be exactly ChunkSize", i+2, maxChunks)
123+
}
124+
125+
// The last chunk just needs to be non-empty.
126+
lastName := extraChunkNames[maxChunks-2]
127+
lastChunk, ok := secretsByName[lastName]
128+
require.True(t, ok, "last chunk Secret %q not found", lastName)
129+
assert.NotEmpty(t, lastChunk, "last chunk must be non-empty")
130+
131+
_, err = drv.Delete(key)
132+
require.NoError(t, err)
133+
}
134+
135+
func deterministicPayload(n int) []byte {
136+
r := rand.New(rand.NewSource(42))
137+
b := make([]byte, n)
138+
// rand.Read always returns len(b), nil
139+
r.Read(b)
140+
return b
141+
}

0 commit comments

Comments
 (0)