Skip to content

Commit af46317

Browse files
lexfreilexfrei
committed
fix(api): log JSON parse error on deprecation walk
Address review feedback from gemini-code-assist on pkg/registry/apps/application/rest.go:1777. When app.Spec.Raw cannot be unmarshalled to map[string]any the deprecation walker now logs a klog.V(2) line naming the kind, name, namespace and the underlying error before returning. Upstream validation still owns rejecting invalid specs (this hook is purely informational), so the log stays at debug verbosity rather than surfacing as a warning that would fire on transient inputs the caller is about to reject anyway. Helper signature changed to (kind, namespace, name string, raw []byte) so the malformed-input path is reachable from existing unit tests without forcing a fake Application wrapper. Signed-off-by: Aleksei Sviridkin <[email protected]>
1 parent 9d9750e commit af46317

2 files changed

Lines changed: 25 additions & 22 deletions

File tree

‎pkg/registry/apps/application/rest.go‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1767,22 +1767,32 @@ func (r *REST) GroupVersionKind(schema.GroupVersion) schema.GroupVersionKind {
17671767
// flat name (nano/micro/.../2xlarge). Returned in the order
17681768
// presets.FindLegacyPresets discovers them. Pure function so tests can
17691769
// assert the output without intercepting klog.
1770-
func deprecationMessagesFor(kindName string, app *appsv1alpha1.Application) []string {
1771-
if app == nil || app.Spec == nil || len(app.Spec.Raw) == 0 {
1770+
//
1771+
// A malformed spec.Raw is reported via a V(2) debug log instead of a
1772+
// hard error: validation upstream of this hook owns rejecting invalid
1773+
// specs, and surfacing the unmarshal failure as a warning here would
1774+
// fire even on transient inputs the caller is about to reject anyway.
1775+
func deprecationMessagesFor(kindName, namespace, name string, raw []byte) []string {
1776+
if len(raw) == 0 {
17721777
return nil
17731778
}
17741779
var spec map[string]any
1775-
if err := json.Unmarshal(app.Spec.Raw, &spec); err != nil {
1780+
if err := json.Unmarshal(raw, &spec); err != nil {
1781+
klog.V(2).Infof("deprecationMessagesFor: skipping %s/%s in %s: spec is not valid JSON: %v",
1782+
kindName, name, namespace, err)
17761783
return nil
17771784
}
1778-
return presets.FormatDeprecationMessages(kindName, app.Namespace, app.Name, presets.FindLegacyPresets(spec))
1785+
return presets.FormatDeprecationMessages(kindName, namespace, name, presets.FindLegacyPresets(spec))
17791786
}
17801787

17811788
// warnLegacyPresets emits a klog warning per deprecation message for
17821789
// the given Application. Non-blocking: the value still renders
17831790
// correctly through cozy-lib legacy aliases.
17841791
func (r *REST) warnLegacyPresets(app *appsv1alpha1.Application) {
1785-
for _, msg := range deprecationMessagesFor(r.kindName, app) {
1792+
if app == nil || app.Spec == nil {
1793+
return
1794+
}
1795+
for _, msg := range deprecationMessagesFor(r.kindName, app.Namespace, app.Name, app.Spec.Raw) {
17861796
klog.Warning(msg)
17871797
}
17881798
}

‎pkg/registry/apps/application/rest_legacy_presets_test.go‎

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,11 @@ func makeApp(name, namespace, rawSpec string) *appsv1alpha1.Application {
3838
}
3939

4040
func TestDeprecationMessagesFor_TopAndNested(t *testing.T) {
41-
app := makeApp("db1", "tenant-x", `{
41+
raw := []byte(`{
4242
"resourcesPreset":"small",
4343
"sub":{"resourcesPreset":"micro"}
4444
}`)
45-
got := deprecationMessagesFor("Postgres", app)
45+
got := deprecationMessagesFor("Postgres", "tenant-x", "db1", raw)
4646
sort.Strings(got)
4747

4848
want := []string{
@@ -55,30 +55,23 @@ func TestDeprecationMessagesFor_TopAndNested(t *testing.T) {
5555
}
5656

5757
func TestDeprecationMessagesFor_NewValuesSilent(t *testing.T) {
58-
app := makeApp("db1", "tenant-x",
59-
`{"resourcesPreset":"t1.small","sub":{"resourcesPreset":"c1.medium"}}`)
60-
if got := deprecationMessagesFor("Postgres", app); len(got) != 0 {
58+
raw := []byte(`{"resourcesPreset":"t1.small","sub":{"resourcesPreset":"c1.medium"}}`)
59+
if got := deprecationMessagesFor("Postgres", "tenant-x", "db1", raw); len(got) != 0 {
6160
t.Errorf("instance-type values must not produce messages, got %v", got)
6261
}
6362
}
6463

65-
func TestDeprecationMessagesFor_NilAndEmpty(t *testing.T) {
66-
if got := deprecationMessagesFor("Postgres", nil); got != nil {
67-
t.Errorf("nil app produced %v", got)
64+
func TestDeprecationMessagesFor_Empty(t *testing.T) {
65+
if got := deprecationMessagesFor("Postgres", "ns", "name", nil); got != nil {
66+
t.Errorf("nil raw produced %v", got)
6867
}
69-
if got := deprecationMessagesFor("Postgres", &appsv1alpha1.Application{}); got != nil {
70-
t.Errorf("app with nil spec produced %v", got)
71-
}
72-
emptyRaw := makeApp("x", "ns", "")
73-
emptyRaw.Spec = &apiextensionsv1.JSON{Raw: []byte("")}
74-
if got := deprecationMessagesFor("Postgres", emptyRaw); got != nil {
75-
t.Errorf("empty raw spec produced %v", got)
68+
if got := deprecationMessagesFor("Postgres", "ns", "name", []byte("")); got != nil {
69+
t.Errorf("empty raw produced %v", got)
7670
}
7771
}
7872

7973
func TestDeprecationMessagesFor_MalformedSpec(t *testing.T) {
80-
app := makeApp("x", "ns", "{not-json")
81-
if got := deprecationMessagesFor("Postgres", app); got != nil {
74+
if got := deprecationMessagesFor("Postgres", "ns", "name", []byte("{not-json")); got != nil {
8275
t.Errorf("malformed spec produced %v", got)
8376
}
8477
}

0 commit comments

Comments
 (0)