Skip to content

Commit 799e484

Browse files
committed
fix CSV unmarshaling error with custom unmarshaler with pretty errors
Signed-off-by: Jordan Keister <[email protected]>
1 parent 7f37952 commit 799e484

File tree

5 files changed

+78
-14
lines changed

5 files changed

+78
-14
lines changed

Makefile

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ GO := go
33
CMDS := $(addprefix bin/, $(shell ls ./cmd | grep -v opm))
44
OPM := $(addprefix bin/, opm)
55
SPECIFIC_UNIT_TEST := $(if $(TEST),-run $(TEST),)
6+
SPECIFIC_SKIP_UNIT_TEST := $(if $(SKIP),-skip $(SKIP),)
67
extra_env := $(GOENV)
78
export PKG := github.com/operator-framework/operator-registry
89
export GIT_COMMIT := $(or $(SOURCE_GIT_COMMIT),$(shell git rev-parse --short HEAD))
@@ -60,7 +61,7 @@ static: build
6061

6162
.PHONY: unit
6263
unit:
63-
$(GO) test -coverprofile=coverage.out $(SPECIFIC_UNIT_TEST) $(TAGS) $(TEST_RACE) -count=1 ./pkg/... ./alpha/...
64+
$(GO) test -coverprofile=coverage.out $(SPECIFIC_UNIT_TEST) $(SPECIFIC_SKIP_UNIT_TEST) $(TAGS) $(TEST_RACE) -count=1 ./pkg/... ./alpha/...
6465

6566
.PHONY: sanity-check
6667
sanity-check:

alpha/declcfg/declcfg.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"errors"
77
"fmt"
88

9+
prettyunmarshaler "github.com/operator-framework/operator-registry/pkg/prettyunmarshaler"
10+
911
"golang.org/x/text/cases"
1012
utilerrors "k8s.io/apimachinery/pkg/util/errors"
1113
"k8s.io/apimachinery/pkg/util/sets"
@@ -107,7 +109,7 @@ func (m *Meta) UnmarshalJSON(blob []byte) error {
107109
// that eat our error type and return a generic error, such that we lose the
108110
// ability to errors.As to get this error on the other side. For now, just return
109111
// a string error that includes the pretty printed message.
110-
return errors.New(newJSONUnmarshalError(blob, err).Pretty())
112+
return errors.New(prettyunmarshaler.NewJSONUnmarshalError(blob, err).Pretty())
111113
}
112114

113115
// TODO: this function ensures we do not break backwards compatibility with

alpha/declcfg/errors.go renamed to pkg/prettyunmarshaler/prettyunmarshaler.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package declcfg
1+
package prettyunmarshaler
22

33
import (
44
"bytes"
@@ -8,29 +8,29 @@ import (
88
"strings"
99
)
1010

11-
type jsonUnmarshalError struct {
11+
type JsonUnmarshalError struct {
1212
data []byte
1313
offset int64
1414
err error
1515
}
1616

17-
func newJSONUnmarshalError(data []byte, err error) *jsonUnmarshalError {
17+
func NewJSONUnmarshalError(data []byte, err error) *JsonUnmarshalError {
1818
var te *json.UnmarshalTypeError
1919
if errors.As(err, &te) {
20-
return &jsonUnmarshalError{data: data, offset: te.Offset, err: te}
20+
return &JsonUnmarshalError{data: data, offset: te.Offset, err: te}
2121
}
2222
var se *json.SyntaxError
2323
if errors.As(err, &se) {
24-
return &jsonUnmarshalError{data: data, offset: se.Offset, err: se}
24+
return &JsonUnmarshalError{data: data, offset: se.Offset, err: se}
2525
}
26-
return &jsonUnmarshalError{data: data, offset: -1, err: err}
26+
return &JsonUnmarshalError{data: data, offset: -1, err: err}
2727
}
2828

29-
func (e *jsonUnmarshalError) Error() string {
29+
func (e *JsonUnmarshalError) Error() string {
3030
return e.err.Error()
3131
}
3232

33-
func (e *jsonUnmarshalError) Pretty() string {
33+
func (e *JsonUnmarshalError) Pretty() string {
3434
if len(e.data) == 0 || e.offset < 0 || e.offset > int64(len(e.data)) {
3535
return e.err.Error()
3636
}

alpha/declcfg/errors_test.go renamed to pkg/prettyunmarshaler/prettyunmarshaler_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package declcfg
1+
package prettyunmarshaler
22

33
import (
44
"encoding/json"
@@ -135,7 +135,7 @@ func TestJsonUnmarshalError(t *testing.T) {
135135
},
136136
} {
137137
t.Run(tc.name, func(t *testing.T) {
138-
actualErr := newJSONUnmarshalError(tc.data, tc.inErr)
138+
actualErr := NewJSONUnmarshalError(tc.data, tc.inErr)
139139
assert.Equal(t, tc.expectErrorString, actualErr.Error())
140140
assert.Equal(t, tc.expectPrettyString, actualErr.Pretty())
141141
})

pkg/registry/csv.go

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@ package registry
22

33
import (
44
"encoding/json"
5+
"errors"
56
"fmt"
6-
"io/ioutil"
77
"os"
88
"path"
99

10+
prettyunmarshaler "github.com/operator-framework/operator-registry/pkg/prettyunmarshaler"
11+
1012
v1 "k8s.io/api/apps/v1"
1113
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1214
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -82,7 +84,7 @@ type ClusterServiceVersion struct {
8284
// ReadCSVFromBundleDirectory tries to parse every YAML file in the directory without inspecting sub-directories and
8385
// returns a CSV. According to the strict one CSV per bundle rule, func returns an error if more than one CSV is found.
8486
func ReadCSVFromBundleDirectory(bundleDir string) (*ClusterServiceVersion, error) {
85-
dirContent, err := ioutil.ReadDir(bundleDir)
87+
dirContent, err := os.ReadDir(bundleDir)
8688
if err != nil {
8789
return nil, fmt.Errorf("error reading bundle directory %s, %v", bundleDir, err)
8890
}
@@ -412,3 +414,62 @@ func (csv *ClusterServiceVersion) GetSubstitutesFor() string {
412414
}
413415
return substitutesFor
414416
}
417+
418+
func (csv *ClusterServiceVersion) UnmarshalJSON(data []byte) error {
419+
if err := csv.UnmarshalTypeMeta(data); err != nil {
420+
return err
421+
}
422+
423+
var m map[string]json.RawMessage
424+
if err := json.Unmarshal(data, &m); err != nil {
425+
return errors.New(prettyunmarshaler.NewJSONUnmarshalError(data, err).Pretty())
426+
}
427+
428+
for k, v := range m {
429+
switch k {
430+
case "spec":
431+
var spec json.RawMessage
432+
if err := json.Unmarshal(v, &spec); err != nil {
433+
return errors.New(prettyunmarshaler.NewJSONUnmarshalError(v, err).Pretty())
434+
}
435+
csv.Spec = spec
436+
}
437+
}
438+
delete(m, "spec")
439+
440+
// remarshal to pull out embedded/inlined fields
441+
newdata, err := json.Marshal(m)
442+
if err != nil {
443+
return err
444+
}
445+
if err := csv.UnmarshalTypeMeta(newdata); err != nil {
446+
return err
447+
}
448+
if err := csv.UnmarshalObjectMeta(newdata); err != nil {
449+
return err
450+
}
451+
452+
return nil
453+
}
454+
455+
func (csv *ClusterServiceVersion) UnmarshalTypeMeta(data []byte) error {
456+
var t metav1.TypeMeta
457+
if err := json.Unmarshal(data, &t); err != nil {
458+
return errors.New(prettyunmarshaler.NewJSONUnmarshalError(data, err).Pretty())
459+
}
460+
csv.TypeMeta = t
461+
462+
return nil
463+
}
464+
465+
func (csv *ClusterServiceVersion) UnmarshalObjectMeta(data []byte) error {
466+
var o struct {
467+
Metadata metav1.ObjectMeta `json:"metadata"`
468+
}
469+
if err := json.Unmarshal(data, &o); err != nil {
470+
return errors.New(prettyunmarshaler.NewJSONUnmarshalError(data, err).Pretty())
471+
}
472+
csv.ObjectMeta = o.Metadata
473+
474+
return nil
475+
}

0 commit comments

Comments
 (0)