Skip to content

feat: modifying error message for subobject #607

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

feat: modifying error message for subobject #607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bc1d6e5
draft for modifying error message
zyy98 Jul 20, 2022
b580325
minor bug
zyy98 Jul 22, 2022
4023a78
refactor test code, but defer function once catch error, can only run…
zyy98 Jul 22, 2022
bc4097c
minor miss
zyy98 Jul 22, 2022
ca0d775
Merge branch 'master' into error_msg
zyy98 Jul 29, 2022
80aa94f
no gvk in subobject
zyy98 Aug 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion go/fn/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,12 @@ func (e *errKubeObjectFields) Error() string {

// errSubObjectFields raises if the SubObject operation panics.
type errSubObjectFields struct {
obj *SubObject
fields []string
}

func (e *errSubObjectFields) Error() string {
return fmt.Sprintf("SubObject has unmatched field type: `%v", strings.Join(e.fields, "/"))
return fmt.Sprintf("SubObject has unmatched field type: `%v`, relative path to parent kubeObject(group=%v, version=%v, kind=%v) is `%v`", strings.Join(e.fields, "/"), e.obj.gvk.Group, e.obj.gvk.Version, e.obj.gvk.Kind, strings.Join(e.obj.nodePath, "/"))
}

type errResultEnd struct {
Expand Down
62 changes: 38 additions & 24 deletions go/fn/object.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func ParseKubeObject(in []byte) (*KubeObject, error) {
func (o *SubObject) GetOrDie(ptr interface{}, fields ...string) {
_, err := o.Get(ptr, fields...)
if err != nil {
panic(errSubObjectFields{fields: fields})
panic(errSubObjectFields{obj: o, fields: fields})
}
}

Expand All @@ -71,7 +71,7 @@ func (o *SubObject) NestedBool(fields ...string) (bool, bool, error) {
func (o *SubObject) NestedBoolOrDie(fields ...string) bool {
val, _, err := o.NestedBool(fields...)
if err != nil {
panic(errSubObjectFields{fields: fields})
panic(errSubObjectFields{obj: o, fields: fields})
}
return val
}
Expand All @@ -88,7 +88,7 @@ func (o *SubObject) NestedString(fields ...string) (string, bool, error) {
func (o *SubObject) NestedStringOrDie(fields ...string) string {
val, _, err := o.NestedString(fields...)
if err != nil {
panic(errSubObjectFields{fields: fields})
panic(errSubObjectFields{obj: o, fields: fields})
}
return val
}
Expand All @@ -105,7 +105,7 @@ func (o *SubObject) NestedFloat64(fields ...string) (float64, bool, error) {
func (o *SubObject) NestedFloat64OrDie(fields ...string) float64 {
val, _, err := o.NestedFloat64(fields...)
if err != nil {
panic(errSubObjectFields{fields: fields})
panic(errSubObjectFields{obj: o, fields: fields})
}
return val
}
Expand All @@ -122,7 +122,7 @@ func (o *SubObject) NestedInt64(fields ...string) (int64, bool, error) {
func (o *SubObject) NestedInt64OrDie(fields ...string) int64 {
val, _, err := o.NestedInt64(fields...)
if err != nil {
panic(errSubObjectFields{fields: fields})
panic(errSubObjectFields{obj: o, fields: fields})
}
return val
}
Expand All @@ -143,7 +143,7 @@ func (o *SubObject) NestedSlice(fields ...string) (SliceSubObjects, bool, error)
}
sliceVal, found, err := mapVariant.GetNestedSlice(fields[len(fields)-1])
if err != nil {
panic(errSubObjectFields{fields: fields})
panic(errSubObjectFields{obj: o, fields: fields})
}
if !found {
return nil, found, nil
Expand All @@ -154,7 +154,7 @@ func (o *SubObject) NestedSlice(fields ...string) (SliceSubObjects, bool, error)
}
var val []*SubObject
for _, obj := range objects {
val = append(val, &SubObject{obj: obj})
val = append(val, &SubObject{obj: obj, gvk: o.gvk, nodePath: append(o.nodePath, fields...)})
}
return val, true, nil
}
Expand All @@ -166,7 +166,7 @@ func (o *SubObject) NestedSlice(fields ...string) (SliceSubObjects, bool, error)
func (o *SubObject) NestedSliceOrDie(fields ...string) SliceSubObjects {
val, _, err := o.NestedSlice(fields...)
if err != nil {
panic(errSubObjectFields{fields: fields})
panic(errSubObjectFields{obj: o, fields: fields})
}
return val
}
Expand All @@ -186,7 +186,7 @@ func (o *SubObject) NestedStringMap(fields ...string) (map[string]string, bool,
func (o *SubObject) NestedStringMapOrDie(fields ...string) map[string]string {
val, _, err := o.NestedStringMap(fields...)
if err != nil {
panic(errSubObjectFields{fields: fields})
panic(errSubObjectFields{obj: o, fields: fields})
}
return val
}
Expand All @@ -206,7 +206,7 @@ func (o *SubObject) NestedStringSlice(fields ...string) ([]string, bool, error)
func (o *SubObject) NestedStringSliceOrDie(fields ...string) []string {
val, _, err := o.NestedStringSlice(fields...)
if err != nil {
panic(errSubObjectFields{fields: fields})
panic(errSubObjectFields{obj: o, fields: fields})
}
return val
}
Expand All @@ -215,7 +215,7 @@ func (o *SubObject) NestedStringSliceOrDie(fields ...string) []string {
// encounters any error.
func (o *SubObject) RemoveNestedFieldOrDie(fields ...string) {
if _, err := o.RemoveNestedField(fields...); err != nil {
panic(errSubObjectFields{fields: fields})
panic(errSubObjectFields{obj: o, fields: fields})
}
}

Expand Down Expand Up @@ -320,7 +320,7 @@ func (o *SubObject) Get(ptr interface{}, fields ...string) (bool, error) {
// It will panic if it encounters any error.
func (o *SubObject) SetOrDie(val interface{}, fields ...string) {
if err := o.SetNestedField(val, fields...); err != nil {
panic(errSubObjectFields{fields: fields})
panic(errSubObjectFields{obj: o, fields: fields})
}
}

Expand Down Expand Up @@ -421,7 +421,7 @@ func (o *SubObject) SetNestedField(val interface{}, fields ...string) error {
func (o *SubObject) SetNestedIntOrDie(value int, fields ...string) {
err := o.SetNestedInt(value, fields...)
if err != nil {
panic(errSubObjectFields{fields: fields})
panic(errSubObjectFields{obj: o, fields: fields})
}
}

Expand All @@ -434,7 +434,7 @@ func (o *SubObject) SetNestedInt(value int, fields ...string) error {
func (o *SubObject) SetNestedBoolOrDie(value bool, fields ...string) {
err := o.SetNestedBool(value, fields...)
if err != nil {
panic(errSubObjectFields{fields: fields})
panic(errSubObjectFields{obj: o, fields: fields})
}
}

Expand All @@ -447,7 +447,7 @@ func (o *SubObject) SetNestedBool(value bool, fields ...string) error {
func (o *SubObject) SetNestedStringOrDie(value string, fields ...string) {
err := o.SetNestedString(value, fields...)
if err != nil {
panic(errSubObjectFields{fields: fields})
panic(errSubObjectFields{obj: o, fields: fields})
}
}

Expand All @@ -460,7 +460,7 @@ func (o *SubObject) SetNestedString(value string, fields ...string) error {
func (o *SubObject) SetNestedStringMapOrDie(value map[string]string, fields ...string) {
err := o.SetNestedStringMap(value, fields...)
if err != nil {
panic(errSubObjectFields{fields: fields})
panic(errSubObjectFields{obj: o, fields: fields})
}
}

Expand All @@ -473,7 +473,7 @@ func (o *SubObject) SetNestedStringMap(value map[string]string, fields ...string
func (o *SubObject) SetNestedStringSliceOrDie(value []string, fields ...string) {
err := o.SetNestedStringSlice(value, fields...)
if err != nil {
panic(errSubObjectFields{fields: fields})
panic(errSubObjectFields{obj: o, fields: fields})
}
}

Expand Down Expand Up @@ -534,7 +534,7 @@ func (o *KubeObject) SetHeadComment(comment string, fields ...string) error {
// be a pointer to a typed object. It will panic if it encounters an error.
func (o *SubObject) AsOrDie(ptr interface{}) {
if err := o.As(ptr); err != nil {
panic(errSubObjectFields{fields: nil})
panic(errSubObjectFields{obj: o, fields: nil})
}
}

Expand Down Expand Up @@ -910,11 +910,22 @@ func (o *KubeObject) IsEmpty() bool {
}

func NewEmptyKubeObject() *KubeObject {
return &KubeObject{SubObject{internal.NewMap(nil)}}
return &KubeObject{SubObject: SubObject{obj: internal.NewMap(nil)}}
}

func asKubeObject(obj *internal.MapVariant) *KubeObject {
return &KubeObject{SubObject{obj}}
kind, _, _ := obj.GetNestedString("kind")
apiVersion, _, _ := obj.GetNestedString("apiVersion")
group, version := ParseGroupVersion(apiVersion)
if kind != "" || group != "" || version != "" {
return &KubeObject{SubObject: SubObject{obj: obj, gvk: &ResourceIdentifier{
Group: group,
Version: version,
Kind: kind,
}}}
}
// will not go to this line as KubeObject contains GVK for sure
return &KubeObject{SubObject: SubObject{obj: obj}}
}

func (o *KubeObject) node() *internal.MapVariant {
Expand All @@ -923,17 +934,20 @@ func (o *KubeObject) node() *internal.MapVariant {

func rnodeToKubeObject(rn *yaml.RNode) *KubeObject {
mapVariant := internal.NewMap(rn.YNode())
return &KubeObject{SubObject{mapVariant}}
return asKubeObject(mapVariant)
//return &KubeObject{SubObject: SubObject{obj: mapVariant}}
}

// SubObject represents a map within a KubeObject
type SubObject struct {
obj *internal.MapVariant
obj *internal.MapVariant
gvk *ResourceIdentifier // the identifier
nodePath []string // the relative path to the kubeObject
}

func (o *SubObject) UpsertMap(k string) *SubObject {
m := o.obj.UpsertMap(k)
return &SubObject{obj: m}
return &SubObject{gvk: o.gvk, nodePath: o.nodePath, obj: m}
}

// GetMap accepts a single key `k` whose value is expected to be a map. It returns
Expand All @@ -945,7 +959,7 @@ func (o *SubObject) GetMap(k string) *SubObject {
if err != nil || !found {
return nil
}
return &SubObject{obj: internal.NewMap(variant.YNode())}
return &SubObject{obj: internal.NewMap(variant.YNode()), gvk: o.gvk, nodePath: append(o.nodePath, k)}
}

// GetBool accepts a single key `k` whose value is expected to be a boolean. It returns
Expand Down
83 changes: 83 additions & 0 deletions go/fn/object_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,89 @@ import (
"github.com/stretchr/testify/assert"
)

// processError recover the error catch by panic
func processError(t *testing.T, expected string, err error) {
v := recover()
if v != nil {
switch t := v.(type) {
case errKubeObjectFields:
err = &t
case *errKubeObjectFields:
err = t
case errSubObjectFields:
err = &t
case *errSubObjectFields:
err = t
case errResultEnd:
err = &t
case *errResultEnd:
err = t
case ErrAttemptToTouchUpstreamIdentifier:
err = &t
case *ErrAttemptToTouchUpstreamIdentifier:
err = t
case ErrInternalAnnotation:
err = &t
case *ErrInternalAnnotation:
err = t
default:
panic(v)
}
}
assert.Equal(t, err.Error(), expected)
}

func TestWrongKRM(t *testing.T) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Write tests separately instead of using parameterized way because recover panic can only catch error once. Not sure if there is a better way to include all test cases about error messaging testing into one Test function.

input := `
apiVersion: apps/v1
kind: StatefulSet
metadata:
name: web
spec:
selector:
matchLabels:
app: nginx # has to match .spec.template.metadata.labels
serviceName: "nginx"
replicas: 3 # by default is 1
minReadySeconds: 10 # by default is 0
volumeClaimTemplates:
- metadata:
name: www
labels:
key: www
`
parseInput, err := ParseKubeObject([]byte(input))
expectedKRMError := "SubObject has unmatched field type: `metadata`, relative path to parent kubeObject(group=apps, version=v1, kind=StatefulSet) is ``"
defer processError(t, expectedKRMError, err)
_, _, err = parseInput.NestedSlice("metadata")
}

func TestWrongKRMWithSubObject(t *testing.T) {
input := `
apiVersion: apps/v1
kind: StatefulSet
metadata:
name: web
spec:
selector:
matchLabels:
app: nginx # has to match .spec.template.metadata.labels
serviceName: "nginx"
replicas: 3 # by default is 1
minReadySeconds: 10 # by default is 0
volumeClaimTemplates:
- metadata:
name: www
labels:
key: www
`
parseInput, err := ParseKubeObject([]byte(input))
expectedKRMError := "SubObject has unmatched field type: `serviceName`, relative path to parent kubeObject(group=apps, version=v1, kind=StatefulSet) is `spec`"
defer processError(t, expectedKRMError, err)
subObj := parseInput.GetMap("spec")
_, _, err = subObj.NestedSlice("serviceName")
}

func TestIsGVK(t *testing.T) {
input := []byte(`
apiVersion: apps/v3
Expand Down
2 changes: 1 addition & 1 deletion go/fn/resourcelist.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func ParseResourceList(in []byte) (*ResourceList, error) {
// toYNode converts the ResourceList to the yaml.Node representation.
func (rl *ResourceList) toYNode() (*yaml.Node, error) {
reMap := internal.NewMap(nil)
reObj := &KubeObject{SubObject{reMap}}
reObj := &KubeObject{SubObject: SubObject{obj: reMap}}
reObj.SetAPIVersion(kio.ResourceListAPIVersion)
reObj.SetKind(kio.ResourceListKind)

Expand Down