Skip to content

Commit

Permalink
Fix panics in Kubernetes plugin (#1157)
Browse files Browse the repository at this point in the history
* Fix panics in Kubernetes plugin
* Fix test assertion
  • Loading branch information
pkosiec authored Jul 20, 2023
1 parent 988389d commit 332a33b
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 5 deletions.
21 changes: 18 additions & 3 deletions internal/source/kubernetes/registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func (r registration) matchEvent(routes []route, event event.Event) (bool, error
errs := multierror.New()
for _, rt := range routes {
// event reason
if rt.event.Reason.AreConstraintsDefined() {
if rt.event != nil && rt.event.Reason.AreConstraintsDefined() {
match, err := rt.event.Reason.IsAllowed(event.Reason)
if err != nil {
return false, err
Expand All @@ -153,7 +153,7 @@ func (r registration) matchEvent(routes []route, event event.Event) (bool, error
}

// event message
if rt.event.Message.AreConstraintsDefined() {
if rt.event != nil && rt.event.Message.AreConstraintsDefined() {
var anyMsgMatches bool

eventMsgs := event.Messages
Expand Down Expand Up @@ -191,7 +191,7 @@ func (r registration) matchEvent(routes []route, event event.Event) (bool, error
}

// namespace
if rt.namespaces.AreConstraintsDefined() {
if rt.namespaces != nil && rt.namespaces.AreConstraintsDefined() {
match, err := rt.namespaces.IsAllowed(event.Namespace)
if err != nil {
return false, err
Expand Down Expand Up @@ -291,6 +291,15 @@ func (r registration) qualifyEventForUpdate(
r.log.Error("Failed to typecast new object to Unstructured.")
}

if oldUnstruct == nil {
r.log.Debugf("oldUnstruct is nil, creating an empty one")
oldUnstruct = &unstructured.Unstructured{}
}
if newUnstruct == nil {
r.log.Debugf("newUnstruct is nil, creating an empty one")
newUnstruct = &unstructured.Unstructured{}
}

var result bool

for _, route := range routes {
Expand All @@ -300,6 +309,12 @@ func (r registration) qualifyEventForUpdate(
continue
}

if route.updateSetting == nil {
// in theory this should never happen, as we check if it is not nil in `route.hasActionableUpdateSetting()`, but just in case
r.log.Debugf("Nil updateSetting but hasActionableUpdateSetting returned true for route: %v. This looks like a bug...", route)
continue
}

r.log.WithFields(logrus.Fields{"old": oldUnstruct.Object, "new": newUnstruct.Object}).Debug("Getting diff for objects...")
diff, err := k8sutil.Diff(oldUnstruct.Object, newUnstruct.Object, *route.updateSetting)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion internal/source/kubernetes/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type route struct {
}

func (r route) hasActionableUpdateSetting() bool {
return len(r.updateSetting.Fields) > 0
return r.updateSetting != nil && len(r.updateSetting.Fields) > 0
}

type entry struct {
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ func assertAliases(t *testing.T, actual []*gqlModel.Alias) {
},
}

assert.Len(t, actual, 2)
assert.Len(t, actual, 4)

// trim ID and deployments
for i := range actual {
Expand Down

0 comments on commit 332a33b

Please sign in to comment.