Skip to content
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.

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

cleanerr: Clean errors for stable status #122

Merged
merged 1 commit into from
Aug 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
37 changes: 37 additions & 0 deletions internal/controller/object/cleanerr.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package object

import "regexp"

// CleanErr masks pointers in err when printing the message.
// errors.As and errors.Is can still find the original error.
// If err is nil, CleanErr returns nil.
func CleanErr(err error) error {
if err == nil {
return nil
}
return &cleanedError{cause: err}
}

type cleanedError struct {
cause error
}

func (w *cleanedError) Error() string { return cleanErrorMessage(w.cause) }
func (w *cleanedError) Cause() error { return w.cause }

// Unwrap provides compatibility for Go 1.13 error chains.
func (w *cleanedError) Unwrap() error { return w.cause }

var pointerRegex = regexp.MustCompile(`\(0x[0-9a-f]{5,}\)`)

// cleanErrorMessage returns the error reproducible error message for kubernetes errors by removing any pointers in the message.
//
// Given
//
// cannot patch object: Job.batch "pi-1" is invalid: spec.template: Invalid value: core.PodTemplateSpec{... TerminationGracePeriodSeconds:(*int64)(0x4012805d98)...: field is immutable
//
// the pointer will be masked: TerminationGracePeriodSeconds:(*int64)(..ptr..)
func cleanErrorMessage(err error) string {
oldString := err.Error()
return pointerRegex.ReplaceAllString(oldString, "(..ptr..)")
}
65 changes: 65 additions & 0 deletions internal/controller/object/cleanerr_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package object

import (
"testing"

"github.com/pkg/errors"
kerrors "k8s.io/apimachinery/pkg/api/errors"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func TestCleanWrap(t *testing.T) {
const (
messageWithoutPointer = "this is 0123abc a normal error message (yeah)"
messageWithPointer = "cannot patch object: Job.batch 'pi-1' is invalid: spec.template: Invalid value: core.PodTemplateSpec{... TerminationGracePeriodSeconds:(*int64)(0x4012805d98): field is immutable"
messageWithPointerMasked = "cannot patch object: Job.batch 'pi-1' is invalid: spec.template: Invalid value: core.PodTemplateSpec{... TerminationGracePeriodSeconds:(*int64)(..ptr..): field is immutable"
)
tests := []struct {
name string
in error

check func(t *testing.T, err error)
}{
{
name: "nil is nil",
in: nil,
check: func(t *testing.T, err error) {
if err != nil {
t.Errorf("expected nil error, got %+v", err)
}
},
},
{
name: "normal error preserved",
in: errors.New(messageWithoutPointer),
check: func(t *testing.T, err error) {
if err.Error() != messageWithoutPointer {
t.Errorf("expected %q error, got %v", messageWithoutPointer, err.Error())
}
},
},
{
name: "pointer is masked",
in: errors.New(messageWithPointer),
check: func(t *testing.T, err error) {
if err.Error() != messageWithPointerMasked {
t.Errorf("expected %q error, got %v", messageWithPointerMasked, err.Error())
}
},
},
{
name: "errors.As preseved and kubernetes compatibility",
in: &kerrors.StatusError{ErrStatus: v1.Status{Reason: v1.StatusReasonAlreadyExists}},
check: func(t *testing.T, err error) {
if !kerrors.IsAlreadyExists(err) {
t.Error("expected kerrors.IsAlreadyExists, got false")
}
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tt.check(t, CleanErr(tt.in))
})
}
}
2 changes: 1 addition & 1 deletion internal/controller/object/object.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext
})

if err := c.client.Apply(ctx, obj); err != nil {
return managed.ExternalUpdate{}, errors.Wrap(err, errApplyObject)
return managed.ExternalUpdate{}, errors.Wrap(CleanErr(err), errApplyObject)
}

return managed.ExternalUpdate{}, c.setObserved(cr, obj)
Expand Down
Loading