Skip to content

Commit aec2345

Browse files
Fix: Truncate large error messages in status conditions
When upgrading operators, CRD validation errors can be very large (50KB+). Kubernetes rejects status updates over 32KB with "Too long: may not be more than 32768 bytes". This causes ClusterExtension upgrades to fail and get stuck. Assisted-by: Cursor
1 parent 182a0a6 commit aec2345

File tree

3 files changed

+201
-11
lines changed

3 files changed

+201
-11
lines changed

internal/operator-controller/controllers/clusterextension_controller.go

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222
"fmt"
2323
"io/fs"
2424
"strings"
25-
"time"
2625

2726
"github.com/go-logr/logr"
2827
"helm.sh/helm/v3/pkg/release"
@@ -156,15 +155,13 @@ func ensureAllConditionsWithReason(ext *ocv1.ClusterExtension, reason v1alpha1.C
156155
cond := apimeta.FindStatusCondition(ext.Status.Conditions, condType)
157156
if cond == nil {
158157
// Create a new condition with a valid reason and add it
159-
newCond := metav1.Condition{
158+
SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
160159
Type: condType,
161160
Status: metav1.ConditionFalse,
162161
Reason: string(reason),
163162
Message: message,
164163
ObservedGeneration: ext.GetGeneration(),
165-
LastTransitionTime: metav1.NewTime(time.Now()),
166-
}
167-
ext.Status.Conditions = append(ext.Status.Conditions, newCond)
164+
})
168165
}
169166
}
170167
}
@@ -381,7 +378,7 @@ func SetDeprecationStatus(ext *ocv1.ClusterExtension, bundleName string, depreca
381378
if len(deprecationMessages) > 0 {
382379
status, reason, message = metav1.ConditionTrue, ocv1.ReasonDeprecated, strings.Join(deprecationMessages, ";")
383380
}
384-
apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
381+
SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
385382
Type: ocv1.TypeDeprecated,
386383
Reason: reason,
387384
Status: status,
@@ -403,7 +400,7 @@ func SetDeprecationStatus(ext *ocv1.ClusterExtension, bundleName string, depreca
403400
message = fmt.Sprintf("%s\n%s", message, entry.Message)
404401
}
405402
}
406-
apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
403+
SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
407404
Type: conditionType,
408405
Reason: reason,
409406
Status: status,

internal/operator-controller/controllers/common_controller.go

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,30 @@ import (
2727
ocv1 "github.com/operator-framework/operator-controller/api/v1"
2828
)
2929

30+
const (
31+
// maxConditionMessageLength is the Kubernetes API limit (32768) less tolerance for truncate suffix length (25 characters)
32+
maxConditionMessageLength = 32000
33+
// truncationSuffix is added when messages are too long
34+
truncationSuffix = "\n\n... [message truncated]"
35+
)
36+
37+
// truncateMessage cuts long messages to fit Kubernetes condition limits
38+
func truncateMessage(message string) string {
39+
if len(message) <= maxConditionMessageLength {
40+
return message
41+
}
42+
43+
maxContent := maxConditionMessageLength - len(truncationSuffix)
44+
return message[:maxContent] + truncationSuffix
45+
}
46+
47+
// SetStatusCondition wraps apimeta.SetStatusCondition and ensures the message is always truncated
48+
// This should be used throughout the codebase instead of apimeta.SetStatusCondition directly
49+
func SetStatusCondition(conditions *[]metav1.Condition, condition metav1.Condition) {
50+
condition.Message = truncateMessage(condition.Message)
51+
apimeta.SetStatusCondition(conditions, condition)
52+
}
53+
3054
// setInstalledStatusFromBundle sets the installed status based on the given installedBundle.
3155
func setInstalledStatusFromBundle(ext *ocv1.ClusterExtension, installedBundle *InstalledBundle) {
3256
// Nothing is installed
@@ -45,7 +69,7 @@ func setInstalledStatusFromBundle(ext *ocv1.ClusterExtension, installedBundle *I
4569

4670
// setInstalledStatusConditionSuccess sets the installed status condition to success.
4771
func setInstalledStatusConditionSuccess(ext *ocv1.ClusterExtension, message string) {
48-
apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
72+
SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
4973
Type: ocv1.TypeInstalled,
5074
Status: metav1.ConditionTrue,
5175
Reason: ocv1.ReasonSucceeded,
@@ -56,7 +80,7 @@ func setInstalledStatusConditionSuccess(ext *ocv1.ClusterExtension, message stri
5680

5781
// setInstalledStatusConditionFailed sets the installed status condition to failed.
5882
func setInstalledStatusConditionFailed(ext *ocv1.ClusterExtension, message string) {
59-
apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
83+
SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
6084
Type: ocv1.TypeInstalled,
6185
Status: metav1.ConditionFalse,
6286
Reason: ocv1.ReasonFailed,
@@ -67,7 +91,7 @@ func setInstalledStatusConditionFailed(ext *ocv1.ClusterExtension, message strin
6791

6892
// setInstalledStatusConditionUnknown sets the installed status condition to unknown.
6993
func setInstalledStatusConditionUnknown(ext *ocv1.ClusterExtension, message string) {
70-
apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
94+
SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
7195
Type: ocv1.TypeInstalled,
7296
Status: metav1.ConditionUnknown,
7397
Reason: ocv1.ReasonFailed,
@@ -99,5 +123,5 @@ func setStatusProgressing(ext *ocv1.ClusterExtension, err error) {
99123
progressingCond.Reason = ocv1.ReasonBlocked
100124
}
101125

102-
apimeta.SetStatusCondition(&ext.Status.Conditions, progressingCond)
126+
SetStatusCondition(&ext.Status.Conditions, progressingCond)
103127
}

internal/operator-controller/controllers/common_controller_test.go

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package controllers
22

33
import (
44
"errors"
5+
"fmt"
6+
"strings"
57
"testing"
68

79
"github.com/google/go-cmp/cmp"
@@ -64,3 +66,170 @@ func TestSetStatusProgressing(t *testing.T) {
6466
})
6567
}
6668
}
69+
70+
func TestTruncateMessage(t *testing.T) {
71+
tests := []struct {
72+
name string
73+
message string
74+
expected string
75+
}{
76+
{
77+
name: "short message unchanged",
78+
message: "This is a short message",
79+
expected: "This is a short message",
80+
},
81+
{
82+
name: "empty message unchanged",
83+
message: "",
84+
expected: "",
85+
},
86+
{
87+
name: "exact max length message unchanged",
88+
message: strings.Repeat("a", maxConditionMessageLength),
89+
expected: strings.Repeat("a", maxConditionMessageLength),
90+
},
91+
{
92+
name: "message just over limit gets truncated",
93+
message: strings.Repeat("a", maxConditionMessageLength+1),
94+
expected: strings.Repeat("a", maxConditionMessageLength-len(truncationSuffix)) + truncationSuffix,
95+
},
96+
{
97+
name: "very long message gets truncated",
98+
message: strings.Repeat("word ", 10000) + "finalword",
99+
expected: strings.Repeat("word ", 10000)[:maxConditionMessageLength-len(truncationSuffix)] + truncationSuffix,
100+
},
101+
}
102+
103+
for _, tc := range tests {
104+
t.Run(tc.name, func(t *testing.T) {
105+
result := truncateMessage(tc.message)
106+
require.Equal(t, tc.expected, result)
107+
108+
// Verify the result is within the limit
109+
require.LessOrEqual(t, len(result), maxConditionMessageLength,
110+
"truncated message should not exceed max length")
111+
112+
// If the original message was over the limit, verify truncation occurred
113+
if len(tc.message) > maxConditionMessageLength {
114+
require.Contains(t, result, truncationSuffix,
115+
"long messages should contain truncation suffix")
116+
require.Less(t, len(result), len(tc.message),
117+
"truncated message should be shorter than original")
118+
}
119+
})
120+
}
121+
}
122+
123+
func TestSetStatusProgressingWithLongMessage(t *testing.T) {
124+
// Create a very long CRD validation error message that exceeds our limit
125+
longCRDError := fmt.Sprintf("validating upgrade for CRD \"applications.argoproj.io\": %s",
126+
strings.Repeat("v1alpha1: ^.spec.generators: unhandled changes found\n", 1000))
127+
128+
ext := &ocv1.ClusterExtension{}
129+
err := errors.New(longCRDError)
130+
setStatusProgressing(ext, err)
131+
132+
cond := meta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeProgressing)
133+
require.NotNil(t, cond)
134+
require.LessOrEqual(t, len(cond.Message), maxConditionMessageLength)
135+
require.Contains(t, cond.Message, truncationSuffix)
136+
require.Contains(t, cond.Message, "validating upgrade for CRD")
137+
}
138+
139+
func TestSetInstalledStatusConditionUnknownWithLongMessage(t *testing.T) {
140+
longError := strings.Repeat("some long error message ", 2000)
141+
142+
ext := &ocv1.ClusterExtension{}
143+
setInstalledStatusConditionUnknown(ext, longError)
144+
145+
cond := meta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeInstalled)
146+
require.NotNil(t, cond)
147+
require.LessOrEqual(t, len(cond.Message), maxConditionMessageLength)
148+
require.Contains(t, cond.Message, truncationSuffix)
149+
}
150+
151+
func TestCRDValidationMessageTruncation(t *testing.T) {
152+
// Test that CRD validation messages get properly truncated
153+
crdError := "validating upgrade for CRD \"applications.argoproj.io\": " +
154+
strings.Repeat("v1alpha1: ^.spec.generators: unhandled changes found in JSONSchemaProps\n", 800)
155+
156+
ext := &ocv1.ClusterExtension{ObjectMeta: metav1.ObjectMeta{Name: "test-extension"}}
157+
err := errors.New(crdError)
158+
setStatusProgressing(ext, err)
159+
160+
cond := meta.FindStatusCondition(ext.Status.Conditions, ocv1.TypeProgressing)
161+
require.NotNil(t, cond)
162+
163+
// Verify message was truncated due to length
164+
require.LessOrEqual(t, len(cond.Message), maxConditionMessageLength)
165+
require.Contains(t, cond.Message, truncationSuffix, "CRD validation messages should be truncated when too long")
166+
require.Contains(t, cond.Message, "validating upgrade for CRD", "should preserve important context")
167+
require.Equal(t, metav1.ConditionTrue, cond.Status)
168+
require.Equal(t, ocv1.ReasonRetrying, cond.Reason)
169+
170+
// Verify original message was actually longer than the limit
171+
require.Greater(t, len(crdError), maxConditionMessageLength, "test should use a message that exceeds the limit")
172+
}
173+
174+
func TestSetStatusConditionWrapper(t *testing.T) {
175+
tests := []struct {
176+
name string
177+
message string
178+
expectedTruncated bool
179+
}{
180+
{
181+
name: "short message not truncated",
182+
message: "This is a short message",
183+
expectedTruncated: false,
184+
},
185+
{
186+
name: "long message gets truncated",
187+
message: strings.Repeat("This is a very long message. ", 2000),
188+
expectedTruncated: true,
189+
},
190+
{
191+
name: "message at exact limit not truncated",
192+
message: strings.Repeat("a", maxConditionMessageLength),
193+
expectedTruncated: false,
194+
},
195+
{
196+
name: "message over limit gets truncated",
197+
message: strings.Repeat("a", maxConditionMessageLength+1),
198+
expectedTruncated: true,
199+
},
200+
}
201+
202+
for _, tc := range tests {
203+
t.Run(tc.name, func(t *testing.T) {
204+
var conditions []metav1.Condition
205+
206+
// Use our wrapper function
207+
SetStatusCondition(&conditions, metav1.Condition{
208+
Type: "TestCondition",
209+
Status: metav1.ConditionTrue,
210+
Reason: "Testing",
211+
Message: tc.message,
212+
})
213+
214+
require.Len(t, conditions, 1, "should have exactly one condition")
215+
cond := conditions[0]
216+
217+
// Verify message is within limits
218+
require.LessOrEqual(t, len(cond.Message), maxConditionMessageLength,
219+
"condition message should not exceed max length")
220+
221+
// Check if truncation occurred as expected
222+
if tc.expectedTruncated {
223+
require.Contains(t, cond.Message, truncationSuffix,
224+
"long messages should contain truncation suffix")
225+
require.Less(t, len(cond.Message), len(tc.message),
226+
"truncated message should be shorter than original")
227+
} else {
228+
require.Equal(t, tc.message, cond.Message,
229+
"short messages should remain unchanged")
230+
require.NotContains(t, cond.Message, truncationSuffix,
231+
"short messages should not contain truncation suffix")
232+
}
233+
})
234+
}
235+
}

0 commit comments

Comments
 (0)