Skip to content

Commit 44cef57

Browse files
enhance to extract details from unhandle issue
1 parent 7df3830 commit 44cef57

File tree

3 files changed

+169
-4
lines changed

3 files changed

+169
-4
lines changed

internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety.go

Lines changed: 133 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"strings"
78

89
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
910
apiextensionsv1client "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1"
@@ -165,7 +166,7 @@ func sameVersionErrors(results *runner.Results) []error {
165166
for _, err := range result.Errors {
166167
msg := err
167168
if result.Name == "unhandled" {
168-
msg = "unhandled changes found"
169+
msg = conciseUnhandledMessage(err)
169170
}
170171
errs = append(errs, fmt.Errorf("%s: %s: %s: %s", version, property, result.Name, msg))
171172
}
@@ -188,7 +189,7 @@ func servedVersionErrors(results *runner.Results) []error {
188189
for _, err := range result.Errors {
189190
msg := err
190191
if result.Name == "unhandled" {
191-
msg = "unhandled changes found"
192+
msg = conciseUnhandledMessage(err)
192193
}
193194
errs = append(errs, fmt.Errorf("%s: %s: %s: %s", version, property, result.Name, msg))
194195
}
@@ -198,3 +199,133 @@ func servedVersionErrors(results *runner.Results) []error {
198199

199200
return errs
200201
}
202+
203+
const unhandledSummaryPrefix = "unhandled changes found"
204+
205+
// conciseUnhandledMessage trims the CRD diff emitted by crdify's "unhandled" comparator
206+
// into a short human readable description so operators get a hint of the change without
207+
// the unreadable Go struct dump.
208+
func conciseUnhandledMessage(raw string) string {
209+
if !strings.Contains(raw, unhandledSummaryPrefix) {
210+
return raw
211+
}
212+
213+
details := extractUnhandledDetails(raw)
214+
if len(details) == 0 {
215+
return unhandledSummaryPrefix
216+
}
217+
218+
return fmt.Sprintf("%s (%s)", unhandledSummaryPrefix, strings.Join(details, "; "))
219+
}
220+
221+
func extractUnhandledDetails(raw string) []string {
222+
type diffEntry struct {
223+
before string
224+
after string
225+
beforeRaw string
226+
afterRaw string
227+
}
228+
229+
entries := map[string]*diffEntry{}
230+
order := []string{}
231+
232+
for _, line := range strings.Split(raw, "\n") {
233+
trimmed := strings.TrimSpace(line)
234+
if len(trimmed) < 2 {
235+
continue
236+
}
237+
238+
sign := trimmed[0]
239+
if sign != '-' && sign != '+' {
240+
continue
241+
}
242+
243+
field, value, rawValue := parseUnhandledDiffValue(trimmed[1:])
244+
if field == "" {
245+
continue
246+
}
247+
248+
entry, ok := entries[field]
249+
if !ok {
250+
entry = &diffEntry{}
251+
entries[field] = entry
252+
order = append(order, field)
253+
}
254+
255+
if sign == '-' {
256+
entry.before = value
257+
entry.beforeRaw = rawValue
258+
} else {
259+
entry.after = value
260+
entry.afterRaw = rawValue
261+
}
262+
}
263+
264+
details := []string{}
265+
for _, field := range order {
266+
entry := entries[field]
267+
if entry.before == "" && entry.after == "" {
268+
continue
269+
}
270+
if entry.before == entry.after && entry.beforeRaw == entry.afterRaw {
271+
continue
272+
}
273+
274+
before := entry.before
275+
if before == "" {
276+
before = "<empty>"
277+
}
278+
after := entry.after
279+
if after == "" {
280+
after = "<empty>"
281+
}
282+
if entry.before == entry.after && entry.beforeRaw != entry.afterRaw {
283+
after = after + " (changed)"
284+
}
285+
286+
details = append(details, fmt.Sprintf("%s %s -> %s", field, before, after))
287+
}
288+
289+
return details
290+
}
291+
292+
func parseUnhandledDiffValue(fragment string) (string, string, string) {
293+
cleaned := strings.TrimSpace(fragment)
294+
cleaned = strings.TrimPrefix(cleaned, "\t")
295+
cleaned = strings.TrimSpace(cleaned)
296+
cleaned = strings.TrimSuffix(cleaned, ",")
297+
298+
parts := strings.SplitN(cleaned, ":", 2)
299+
if len(parts) != 2 {
300+
return "", "", ""
301+
}
302+
303+
field := strings.TrimSpace(parts[0])
304+
rawValue := strings.TrimSpace(parts[1])
305+
value := normalizeUnhandledValue(rawValue)
306+
307+
if field == "" {
308+
return "", "", ""
309+
}
310+
311+
return field, value, rawValue
312+
}
313+
314+
func normalizeUnhandledValue(value string) string {
315+
value = strings.TrimSuffix(value, ",")
316+
value = strings.TrimSpace(value)
317+
318+
switch value {
319+
case "":
320+
return "<empty>"
321+
case "\"\"":
322+
return "\"\""
323+
}
324+
325+
value = strings.ReplaceAll(value, "v1.", "")
326+
if strings.Contains(value, "JSONSchemaProps") {
327+
return "<complex value>"
328+
}
329+
330+
return value
331+
}

internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -395,9 +395,12 @@ func TestUpgrade_UnhandledChanges_InSpec_DefaultPolicy(t *testing.T) {
395395
Name: "test-release",
396396
Manifest: getManifestString(t, "crd-unhandled-new.json"),
397397
}
398-
err := preflight.Upgrade(context.Background(), rel)
398+
objs, err := applier.HelmReleaseToObjectsConverter{}.GetObjectsFromRelease(rel)
399+
require.NoError(t, err)
400+
err = preflight.Upgrade(context.Background(), objs)
399401
require.Error(t, err)
400402
require.ErrorContains(t, err, "unhandled changes found")
403+
require.ErrorContains(t, err, "Format \"\" -> \"email\"")
401404
require.NotContains(t, err.Error(), "v1.JSONSchemaProps", "error message should be concise without raw diff")
402405
})
403406
}
@@ -415,8 +418,11 @@ func TestUpgrade_UnhandledChanges_PolicyError(t *testing.T) {
415418
Manifest: getManifestString(t, "crd-unhandled-new.json"),
416419
}
417420

418-
err := preflight.Upgrade(context.Background(), rel)
421+
objs, err := applier.HelmReleaseToObjectsConverter{}.GetObjectsFromRelease(rel)
422+
require.NoError(t, err)
423+
err = preflight.Upgrade(context.Background(), objs)
419424
require.Error(t, err)
420425
require.ErrorContains(t, err, "unhandled changes found")
426+
require.ErrorContains(t, err, "Format \"\" -> \"email\"")
421427
})
422428
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package crdupgradesafety
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
)
8+
9+
func TestConciseUnhandledMessage_NoPrefix(t *testing.T) {
10+
raw := "some other error"
11+
require.Equal(t, raw, conciseUnhandledMessage(raw))
12+
}
13+
14+
func TestConciseUnhandledMessage_SingleChange(t *testing.T) {
15+
raw := "unhandled changes found :\n- Format: \"\"\n+ Format: \"email\"\n"
16+
require.Equal(t, "unhandled changes found (Format \"\" -> \"email\")", conciseUnhandledMessage(raw))
17+
}
18+
19+
func TestConciseUnhandledMessage_MultipleChanges(t *testing.T) {
20+
raw := "unhandled changes found :\n- Format: \"\"\n+ Format: \"email\"\n- Default: nil\n+ Default: \"value\"\n- Title: \"\"\n+ Title: \"Widget\"\n- Description: \"old\"\n+ Description: \"new\"\n"
21+
got := conciseUnhandledMessage(raw)
22+
require.Equal(t, "unhandled changes found (Format \"\" -> \"email\"; Default nil -> \"value\"; Title \"\" -> \"Widget\"; Description \"old\" -> \"new\")", got)
23+
}
24+
25+
func TestConciseUnhandledMessage_SkipComplexValues(t *testing.T) {
26+
raw := "unhandled changes found :\n- Default: &v1.JSONSchemaProps{}\n+ Default: &v1.JSONSchemaProps{Type: \"string\"}\n"
27+
require.Equal(t, "unhandled changes found (Default <complex value> -> <complex value> (changed))", conciseUnhandledMessage(raw))
28+
}

0 commit comments

Comments
 (0)