From 576803e4cc61161629cdb39232592982ba30338f Mon Sep 17 00:00:00 2001 From: Jordan Keister Date: Fri, 25 Aug 2023 10:15:54 -0500 Subject: [PATCH] fix CSV unmarshaling error with custom unmarshaler with pretty errors (#1138) Signed-off-by: Jordan Keister --- .gitignore | 3 + staging/operator-registry/Makefile | 3 +- .../alpha/declcfg/declcfg.go | 4 +- .../prettyunmarshaler/prettyunmarshaler.go} | 16 ++--- .../prettyunmarshaler_test.go} | 4 +- staging/operator-registry/pkg/registry/csv.go | 59 ++++++++++++++++++- .../alpha/declcfg/declcfg.go | 4 +- .../prettyunmarshaler/prettyunmarshaler.go} | 16 ++--- .../operator-registry/pkg/registry/csv.go | 59 ++++++++++++++++++- vendor/modules.txt | 1 + 10 files changed, 144 insertions(+), 25 deletions(-) rename staging/operator-registry/{alpha/declcfg/errors.go => pkg/prettyunmarshaler/prettyunmarshaler.go} (83%) rename staging/operator-registry/{alpha/declcfg/errors_test.go => pkg/prettyunmarshaler/prettyunmarshaler_test.go} (98%) rename vendor/github.com/operator-framework/operator-registry/{alpha/declcfg/errors.go => pkg/prettyunmarshaler/prettyunmarshaler.go} (83%) diff --git a/.gitignore b/.gitignore index 38a66a2cd2..ad56576535 100644 --- a/.gitignore +++ b/.gitignore @@ -468,3 +468,6 @@ scripts/**/*.crc.e2e.patch.yaml # downstream sync sha files *.cherrypick + +# ignore temp artifacts of building images +vendor/github.com/operator-framework/operator-registry/pkg/lib/indexer/index.Dockerfile* diff --git a/staging/operator-registry/Makefile b/staging/operator-registry/Makefile index df4ac5072b..8db4b4333e 100644 --- a/staging/operator-registry/Makefile +++ b/staging/operator-registry/Makefile @@ -3,6 +3,7 @@ GO := go CMDS := $(addprefix bin/, $(shell ls ./cmd | grep -v opm)) OPM := $(addprefix bin/, opm) SPECIFIC_UNIT_TEST := $(if $(TEST),-run $(TEST),) +SPECIFIC_SKIP_UNIT_TEST := $(if $(SKIP),-skip $(SKIP),) extra_env := $(GOENV) export PKG := github.com/operator-framework/operator-registry export GIT_COMMIT := $(or $(SOURCE_GIT_COMMIT),$(shell git rev-parse --short HEAD)) @@ -60,7 +61,7 @@ static: build .PHONY: unit unit: - $(GO) test -coverprofile=coverage.out $(SPECIFIC_UNIT_TEST) $(TAGS) $(TEST_RACE) -count=1 ./pkg/... ./alpha/... + $(GO) test -coverprofile=coverage.out $(SPECIFIC_UNIT_TEST) $(SPECIFIC_SKIP_UNIT_TEST) $(TAGS) $(TEST_RACE) -count=1 ./pkg/... ./alpha/... .PHONY: sanity-check sanity-check: diff --git a/staging/operator-registry/alpha/declcfg/declcfg.go b/staging/operator-registry/alpha/declcfg/declcfg.go index 994d47a0b7..f688574b1c 100644 --- a/staging/operator-registry/alpha/declcfg/declcfg.go +++ b/staging/operator-registry/alpha/declcfg/declcfg.go @@ -6,6 +6,8 @@ import ( "errors" "fmt" + prettyunmarshaler "github.com/operator-framework/operator-registry/pkg/prettyunmarshaler" + "golang.org/x/text/cases" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/sets" @@ -107,7 +109,7 @@ func (m *Meta) UnmarshalJSON(blob []byte) error { // that eat our error type and return a generic error, such that we lose the // ability to errors.As to get this error on the other side. For now, just return // a string error that includes the pretty printed message. - return errors.New(newJSONUnmarshalError(blob, err).Pretty()) + return errors.New(prettyunmarshaler.NewJSONUnmarshalError(blob, err).Pretty()) } // TODO: this function ensures we do not break backwards compatibility with diff --git a/staging/operator-registry/alpha/declcfg/errors.go b/staging/operator-registry/pkg/prettyunmarshaler/prettyunmarshaler.go similarity index 83% rename from staging/operator-registry/alpha/declcfg/errors.go rename to staging/operator-registry/pkg/prettyunmarshaler/prettyunmarshaler.go index f5ef115233..2f740151a3 100644 --- a/staging/operator-registry/alpha/declcfg/errors.go +++ b/staging/operator-registry/pkg/prettyunmarshaler/prettyunmarshaler.go @@ -1,4 +1,4 @@ -package declcfg +package prettyunmarshaler import ( "bytes" @@ -8,29 +8,29 @@ import ( "strings" ) -type jsonUnmarshalError struct { +type JsonUnmarshalError struct { data []byte offset int64 err error } -func newJSONUnmarshalError(data []byte, err error) *jsonUnmarshalError { +func NewJSONUnmarshalError(data []byte, err error) *JsonUnmarshalError { var te *json.UnmarshalTypeError if errors.As(err, &te) { - return &jsonUnmarshalError{data: data, offset: te.Offset, err: te} + return &JsonUnmarshalError{data: data, offset: te.Offset, err: te} } var se *json.SyntaxError if errors.As(err, &se) { - return &jsonUnmarshalError{data: data, offset: se.Offset, err: se} + return &JsonUnmarshalError{data: data, offset: se.Offset, err: se} } - return &jsonUnmarshalError{data: data, offset: -1, err: err} + return &JsonUnmarshalError{data: data, offset: -1, err: err} } -func (e *jsonUnmarshalError) Error() string { +func (e *JsonUnmarshalError) Error() string { return e.err.Error() } -func (e *jsonUnmarshalError) Pretty() string { +func (e *JsonUnmarshalError) Pretty() string { if len(e.data) == 0 || e.offset < 0 || e.offset > int64(len(e.data)) { return e.err.Error() } diff --git a/staging/operator-registry/alpha/declcfg/errors_test.go b/staging/operator-registry/pkg/prettyunmarshaler/prettyunmarshaler_test.go similarity index 98% rename from staging/operator-registry/alpha/declcfg/errors_test.go rename to staging/operator-registry/pkg/prettyunmarshaler/prettyunmarshaler_test.go index 9da8f86b91..a322873193 100644 --- a/staging/operator-registry/alpha/declcfg/errors_test.go +++ b/staging/operator-registry/pkg/prettyunmarshaler/prettyunmarshaler_test.go @@ -1,4 +1,4 @@ -package declcfg +package prettyunmarshaler import ( "encoding/json" @@ -135,7 +135,7 @@ func TestJsonUnmarshalError(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - actualErr := newJSONUnmarshalError(tc.data, tc.inErr) + actualErr := NewJSONUnmarshalError(tc.data, tc.inErr) assert.Equal(t, tc.expectErrorString, actualErr.Error()) assert.Equal(t, tc.expectPrettyString, actualErr.Pretty()) }) diff --git a/staging/operator-registry/pkg/registry/csv.go b/staging/operator-registry/pkg/registry/csv.go index ec6f3e3239..8dcdf65adb 100644 --- a/staging/operator-registry/pkg/registry/csv.go +++ b/staging/operator-registry/pkg/registry/csv.go @@ -2,11 +2,13 @@ package registry import ( "encoding/json" + "errors" "fmt" - "io/ioutil" "os" "path" + prettyunmarshaler "github.com/operator-framework/operator-registry/pkg/prettyunmarshaler" + v1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -82,7 +84,7 @@ type ClusterServiceVersion struct { // ReadCSVFromBundleDirectory tries to parse every YAML file in the directory without inspecting sub-directories and // returns a CSV. According to the strict one CSV per bundle rule, func returns an error if more than one CSV is found. func ReadCSVFromBundleDirectory(bundleDir string) (*ClusterServiceVersion, error) { - dirContent, err := ioutil.ReadDir(bundleDir) + dirContent, err := os.ReadDir(bundleDir) if err != nil { return nil, fmt.Errorf("error reading bundle directory %s, %v", bundleDir, err) } @@ -412,3 +414,56 @@ func (csv *ClusterServiceVersion) GetSubstitutesFor() string { } return substitutesFor } + +func (csv *ClusterServiceVersion) UnmarshalJSON(data []byte) error { + + if err := csv.UnmarshalSpec(data); err != nil { + return err + } + if err := csv.UnmarshalTypeMeta(data); err != nil { + return err + } + if err := csv.UnmarshalObjectMeta(data); err != nil { + return err + } + + return nil +} + +func (csv *ClusterServiceVersion) UnmarshalSpec(data []byte) error { + var m map[string]json.RawMessage + if err := json.Unmarshal(data, &m); err != nil { + return errors.New(prettyunmarshaler.NewJSONUnmarshalError(data, err).Pretty()) + } + + spec, ok := m["spec"] + if !ok { + return nil + } + csv.Spec = spec + + return nil +} + +func (csv *ClusterServiceVersion) UnmarshalTypeMeta(data []byte) error { + var t metav1.TypeMeta + if err := json.Unmarshal(data, &t); err != nil { + return errors.New(prettyunmarshaler.NewJSONUnmarshalError(data, err).Pretty()) + } + csv.TypeMeta = t + + return nil +} + +func (csv *ClusterServiceVersion) UnmarshalObjectMeta(data []byte) error { + var o struct { + Metadata metav1.ObjectMeta `json:"metadata"` + } + if err := json.Unmarshal(data, &o); err != nil { + return errors.New(prettyunmarshaler.NewJSONUnmarshalError(data, err).Pretty()) + } + + csv.ObjectMeta = o.Metadata + + return nil +} diff --git a/vendor/github.com/operator-framework/operator-registry/alpha/declcfg/declcfg.go b/vendor/github.com/operator-framework/operator-registry/alpha/declcfg/declcfg.go index 994d47a0b7..f688574b1c 100644 --- a/vendor/github.com/operator-framework/operator-registry/alpha/declcfg/declcfg.go +++ b/vendor/github.com/operator-framework/operator-registry/alpha/declcfg/declcfg.go @@ -6,6 +6,8 @@ import ( "errors" "fmt" + prettyunmarshaler "github.com/operator-framework/operator-registry/pkg/prettyunmarshaler" + "golang.org/x/text/cases" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/sets" @@ -107,7 +109,7 @@ func (m *Meta) UnmarshalJSON(blob []byte) error { // that eat our error type and return a generic error, such that we lose the // ability to errors.As to get this error on the other side. For now, just return // a string error that includes the pretty printed message. - return errors.New(newJSONUnmarshalError(blob, err).Pretty()) + return errors.New(prettyunmarshaler.NewJSONUnmarshalError(blob, err).Pretty()) } // TODO: this function ensures we do not break backwards compatibility with diff --git a/vendor/github.com/operator-framework/operator-registry/alpha/declcfg/errors.go b/vendor/github.com/operator-framework/operator-registry/pkg/prettyunmarshaler/prettyunmarshaler.go similarity index 83% rename from vendor/github.com/operator-framework/operator-registry/alpha/declcfg/errors.go rename to vendor/github.com/operator-framework/operator-registry/pkg/prettyunmarshaler/prettyunmarshaler.go index f5ef115233..2f740151a3 100644 --- a/vendor/github.com/operator-framework/operator-registry/alpha/declcfg/errors.go +++ b/vendor/github.com/operator-framework/operator-registry/pkg/prettyunmarshaler/prettyunmarshaler.go @@ -1,4 +1,4 @@ -package declcfg +package prettyunmarshaler import ( "bytes" @@ -8,29 +8,29 @@ import ( "strings" ) -type jsonUnmarshalError struct { +type JsonUnmarshalError struct { data []byte offset int64 err error } -func newJSONUnmarshalError(data []byte, err error) *jsonUnmarshalError { +func NewJSONUnmarshalError(data []byte, err error) *JsonUnmarshalError { var te *json.UnmarshalTypeError if errors.As(err, &te) { - return &jsonUnmarshalError{data: data, offset: te.Offset, err: te} + return &JsonUnmarshalError{data: data, offset: te.Offset, err: te} } var se *json.SyntaxError if errors.As(err, &se) { - return &jsonUnmarshalError{data: data, offset: se.Offset, err: se} + return &JsonUnmarshalError{data: data, offset: se.Offset, err: se} } - return &jsonUnmarshalError{data: data, offset: -1, err: err} + return &JsonUnmarshalError{data: data, offset: -1, err: err} } -func (e *jsonUnmarshalError) Error() string { +func (e *JsonUnmarshalError) Error() string { return e.err.Error() } -func (e *jsonUnmarshalError) Pretty() string { +func (e *JsonUnmarshalError) Pretty() string { if len(e.data) == 0 || e.offset < 0 || e.offset > int64(len(e.data)) { return e.err.Error() } diff --git a/vendor/github.com/operator-framework/operator-registry/pkg/registry/csv.go b/vendor/github.com/operator-framework/operator-registry/pkg/registry/csv.go index ec6f3e3239..8dcdf65adb 100644 --- a/vendor/github.com/operator-framework/operator-registry/pkg/registry/csv.go +++ b/vendor/github.com/operator-framework/operator-registry/pkg/registry/csv.go @@ -2,11 +2,13 @@ package registry import ( "encoding/json" + "errors" "fmt" - "io/ioutil" "os" "path" + prettyunmarshaler "github.com/operator-framework/operator-registry/pkg/prettyunmarshaler" + v1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -82,7 +84,7 @@ type ClusterServiceVersion struct { // ReadCSVFromBundleDirectory tries to parse every YAML file in the directory without inspecting sub-directories and // returns a CSV. According to the strict one CSV per bundle rule, func returns an error if more than one CSV is found. func ReadCSVFromBundleDirectory(bundleDir string) (*ClusterServiceVersion, error) { - dirContent, err := ioutil.ReadDir(bundleDir) + dirContent, err := os.ReadDir(bundleDir) if err != nil { return nil, fmt.Errorf("error reading bundle directory %s, %v", bundleDir, err) } @@ -412,3 +414,56 @@ func (csv *ClusterServiceVersion) GetSubstitutesFor() string { } return substitutesFor } + +func (csv *ClusterServiceVersion) UnmarshalJSON(data []byte) error { + + if err := csv.UnmarshalSpec(data); err != nil { + return err + } + if err := csv.UnmarshalTypeMeta(data); err != nil { + return err + } + if err := csv.UnmarshalObjectMeta(data); err != nil { + return err + } + + return nil +} + +func (csv *ClusterServiceVersion) UnmarshalSpec(data []byte) error { + var m map[string]json.RawMessage + if err := json.Unmarshal(data, &m); err != nil { + return errors.New(prettyunmarshaler.NewJSONUnmarshalError(data, err).Pretty()) + } + + spec, ok := m["spec"] + if !ok { + return nil + } + csv.Spec = spec + + return nil +} + +func (csv *ClusterServiceVersion) UnmarshalTypeMeta(data []byte) error { + var t metav1.TypeMeta + if err := json.Unmarshal(data, &t); err != nil { + return errors.New(prettyunmarshaler.NewJSONUnmarshalError(data, err).Pretty()) + } + csv.TypeMeta = t + + return nil +} + +func (csv *ClusterServiceVersion) UnmarshalObjectMeta(data []byte) error { + var o struct { + Metadata metav1.ObjectMeta `json:"metadata"` + } + if err := json.Unmarshal(data, &o); err != nil { + return errors.New(prettyunmarshaler.NewJSONUnmarshalError(data, err).Pretty()) + } + + csv.ObjectMeta = o.Metadata + + return nil +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 572308f4ba..4b0c6ae48d 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -805,6 +805,7 @@ github.com/operator-framework/operator-registry/pkg/lib/semver github.com/operator-framework/operator-registry/pkg/lib/tmp github.com/operator-framework/operator-registry/pkg/lib/validation github.com/operator-framework/operator-registry/pkg/mirror +github.com/operator-framework/operator-registry/pkg/prettyunmarshaler github.com/operator-framework/operator-registry/pkg/registry github.com/operator-framework/operator-registry/pkg/server github.com/operator-framework/operator-registry/pkg/sqlite