Skip to content

Commit 56771f7

Browse files
authored
fix CSV unmarshaling error with custom unmarshaler with pretty errors (#1138)
Signed-off-by: Jordan Keister <[email protected]>
1 parent 7f37952 commit 56771f7

File tree

5 files changed

+72
-14
lines changed

5 files changed

+72
-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: 57 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,56 @@ func (csv *ClusterServiceVersion) GetSubstitutesFor() string {
412414
}
413415
return substitutesFor
414416
}
417+
418+
func (csv *ClusterServiceVersion) UnmarshalJSON(data []byte) error {
419+
420+
if err := csv.UnmarshalSpec(data); err != nil {
421+
return err
422+
}
423+
if err := csv.UnmarshalTypeMeta(data); err != nil {
424+
return err
425+
}
426+
if err := csv.UnmarshalObjectMeta(data); err != nil {
427+
return err
428+
}
429+
430+
return nil
431+
}
432+
433+
func (csv *ClusterServiceVersion) UnmarshalSpec(data []byte) error {
434+
var m map[string]json.RawMessage
435+
if err := json.Unmarshal(data, &m); err != nil {
436+
return errors.New(prettyunmarshaler.NewJSONUnmarshalError(data, err).Pretty())
437+
}
438+
439+
spec, ok := m["spec"]
440+
if !ok {
441+
return nil
442+
}
443+
csv.Spec = spec
444+
445+
return nil
446+
}
447+
448+
func (csv *ClusterServiceVersion) UnmarshalTypeMeta(data []byte) error {
449+
var t metav1.TypeMeta
450+
if err := json.Unmarshal(data, &t); err != nil {
451+
return errors.New(prettyunmarshaler.NewJSONUnmarshalError(data, err).Pretty())
452+
}
453+
csv.TypeMeta = t
454+
455+
return nil
456+
}
457+
458+
func (csv *ClusterServiceVersion) UnmarshalObjectMeta(data []byte) error {
459+
var o struct {
460+
Metadata metav1.ObjectMeta `json:"metadata"`
461+
}
462+
if err := json.Unmarshal(data, &o); err != nil {
463+
return errors.New(prettyunmarshaler.NewJSONUnmarshalError(data, err).Pretty())
464+
}
465+
466+
csv.ObjectMeta = o.Metadata
467+
468+
return nil
469+
}

0 commit comments

Comments
 (0)