Ensure diff don't stop on error

kubeshop · Jul 5, 2023 · 8e111ce · 8e111ce
1 parent d4feb29
commit 8e111ce
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 22 deletions.
diff --git a/internal/executor/x/output/message_parser.go b/internal/executor/x/output/message_parser.go
@@ -100,7 +100,7 @@ func (p *TableCommandParser) renderActions(msgCtx template.ParseMessage, table p
 
 	return api.Section{
 		Buttons: []api.Button{
-			btnBuilder.ForCommand("Raw output", fmt.Sprintf("x run %s %s", cmd, x.RawOutputIndicator)),
+			btnBuilder.ForCommandWithoutDesc("Raw output", fmt.Sprintf("x run %s %s", cmd, x.RawOutputIndicator)),
 		},
 		Selects: api.Selects{
 			Items: []api.Select{

diff --git a/internal/source/kubernetes/k8sutil/diff.go b/internal/source/kubernetes/k8sutil/diff.go
@@ -8,22 +8,30 @@ import (
 	"k8s.io/kubectl/pkg/cmd/get"
 
 	"github.com/kubeshop/botkube/internal/source/kubernetes/config"
+	"github.com/kubeshop/botkube/pkg/multierror"
 )
 
 // Diff provides differences between two objects.
 func Diff(x, y interface{}, updateSetting config.UpdateSetting) (string, error) {
 	strBldr := new(strings.Builder)
+
+	errs := multierror.New()
 	for _, val := range updateSetting.Fields {
 		var d diffReporter
 		d.field = val
 		diff, err := d.exec(x, y)
 		if err != nil {
-			return "", err
+			errs = multierror.Append(errs, err)
+			continue
 		}
 
 		strBldr.WriteString(diff)
 	}
 
+	if errs.ErrorOrNil() != nil {
+		return strBldr.String(), fmt.Errorf("while getting diff: %w", errs.ErrorOrNil())
+	}
+
 	return strBldr.String(), nil
 }
 
@@ -35,12 +43,12 @@ func (d diffReporter) exec(x, y interface{}) (string, error) {
 	vx, err := parseJsonpath(x, d.field)
 	if err != nil {
 		// Happens when the fields were not set by the time event was issued, do not return in that case
-		return "", fmt.Errorf("while finding value from jsonpath: %q, object: %+v: %w", d.field, x, err)
+		return "", fmt.Errorf("while finding value in old obj from jsonpath %q: %w", d.field, err)
 	}
 
 	vy, err := parseJsonpath(y, d.field)
 	if err != nil {
-		return "", fmt.Errorf("while finding value from jsonpath: %q, object: %+v: %w", d.field, y, err)
+		return "", fmt.Errorf("while finding value in new obj from jsonpath %q: %w", d.field, err)
 	}
 
 	// treat <none> and false as same fields

diff --git a/internal/source/kubernetes/k8sutil/diff_test.go b/internal/source/kubernetes/k8sutil/diff_test.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/MakeNowJust/heredoc"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -78,10 +79,12 @@ func TestDiff(t *testing.T) {
 			},
 		},
 		`Non Spec Diff`: {
-			old:                Object{Spec: Spec{Containers: []Container{{Image: "nginx:1.14"}}}, Other: Other{Foo: "bar"}},
-			new:                Object{Spec: Spec{Containers: []Container{{Image: "nginx:1.14"}}}, Other: Other{Foo: "boo"}},
-			update:             config.UpdateSetting{Fields: []string{"metadata.name"}, IncludeDiff: true},
-			expectedErrMessage: "while finding value from jsonpath: \"metadata.name\", object: {Spec:{Port:0 Containers:[{Image:nginx:1.14}]} Status:{Replicas:0} Data:{Properties:} Rules:{Verbs:} Other:{Foo:bar Annotations:map[]}}: metadata is not found",
+			old:    Object{Spec: Spec{Containers: []Container{{Image: "nginx:1.14"}}}, Other: Other{Foo: "bar"}},
+			new:    Object{Spec: Spec{Containers: []Container{{Image: "nginx:1.14"}}}, Other: Other{Foo: "boo"}},
+			update: config.UpdateSetting{Fields: []string{"metadata.name"}, IncludeDiff: true},
+			expectedErrMessage: heredoc.Doc(`
+				while getting diff: 1 error occurred:
+					* while finding value in old obj from jsonpath "metadata.name": metadata is not found`),
 		},
 		`Annotations changed`: {
 			old:    Object{Other: Other{Annotations: map[string]string{"app.kubernetes.io/version": "1"}}},
@@ -103,11 +106,26 @@ func TestDiff(t *testing.T) {
 				Y:    "2",
 			},
 		},
+		`Get all diffs even if one of them return errors`: {
+			old:    Object{Status: Status{Replicas: 1}, Other: Other{Foo: "bar"}},
+			new:    Object{Status: Status{Replicas: 2}, Other: Other{Foo: "bar"}},
+			update: config.UpdateSetting{Fields: []string{"status.foo", "status.replicas"}, IncludeDiff: true},
+			expected: ExpectedDiff{
+				Path: "status.replicas",
+				X:    "1",
+				Y:    "2",
+			},
+			expectedErrMessage: heredoc.Doc(`
+				while getting diff: 1 error occurred:
+					* while finding value in old obj from jsonpath "status.foo": foo is not found`),
+		},
 		`Non Status Diff`: {
-			old:                Object{Status: Status{Replicas: 1}, Other: Other{Foo: "bar"}},
-			new:                Object{Status: Status{Replicas: 1}, Other: Other{Foo: "boo"}},
-			update:             config.UpdateSetting{Fields: []string{"metadata.labels"}, IncludeDiff: true},
-			expectedErrMessage: "while finding value from jsonpath: \"metadata.labels\", object: {Spec:{Port:0 Containers:[]} Status:{Replicas:1} Data:{Properties:} Rules:{Verbs:} Other:{Foo:bar Annotations:map[]}}: metadata is not found",
+			old:    Object{Status: Status{Replicas: 1}, Other: Other{Foo: "bar"}},
+			new:    Object{Status: Status{Replicas: 1}, Other: Other{Foo: "boo"}},
+			update: config.UpdateSetting{Fields: []string{"metadata.labels"}, IncludeDiff: true},
+			expectedErrMessage: heredoc.Doc(`
+				while getting diff: 1 error occurred:
+					* while finding value in old obj from jsonpath "metadata.labels": metadata is not found`),
 		},
 		`Event Diff`: {
 			old:    Object{Data: Data{Properties: "color: blue"}, Other: Other{Foo: "bar"}},
@@ -120,10 +138,12 @@ func TestDiff(t *testing.T) {
 			},
 		},
 		`Non Event Diff`: {
-			old:                Object{Data: Data{Properties: "color: blue"}, Other: Other{Foo: "bar"}},
-			new:                Object{Data: Data{Properties: "color: blue"}, Other: Other{Foo: "boo"}},
-			update:             config.UpdateSetting{Fields: []string{"metadata.name"}, IncludeDiff: true},
-			expectedErrMessage: "while finding value from jsonpath: \"metadata.name\", object: {Spec:{Port:0 Containers:[]} Status:{Replicas:0} Data:{Properties:color: blue} Rules:{Verbs:} Other:{Foo:bar Annotations:map[]}}: metadata is not found",
+			old:    Object{Data: Data{Properties: "color: blue"}, Other: Other{Foo: "bar"}},
+			new:    Object{Data: Data{Properties: "color: blue"}, Other: Other{Foo: "boo"}},
+			update: config.UpdateSetting{Fields: []string{"metadata.name"}, IncludeDiff: true},
+			expectedErrMessage: heredoc.Doc(`
+				while getting diff: 1 error occurred:
+					* while finding value in old obj from jsonpath "metadata.name": metadata is not found`),
 		},
 		`Rules Diff`: {
 			old:    Object{Rules: Rules{Verbs: "list"}, Other: Other{Foo: "bar"}},
@@ -136,10 +156,12 @@ func TestDiff(t *testing.T) {
 			},
 		},
 		`Non Rules Diff`: {
-			old:                Object{Rules: Rules{Verbs: "list"}, Other: Other{Foo: "bar"}},
-			new:                Object{Rules: Rules{Verbs: "list"}, Other: Other{Foo: "boo"}},
-			update:             config.UpdateSetting{Fields: []string{"metadata.name"}, IncludeDiff: true},
-			expectedErrMessage: "while finding value from jsonpath: \"metadata.name\", object: {Spec:{Port:0 Containers:[]} Status:{Replicas:0} Data:{Properties:} Rules:{Verbs:list} Other:{Foo:bar Annotations:map[]}}: metadata is not found",
+			old:    Object{Rules: Rules{Verbs: "list"}, Other: Other{Foo: "bar"}},
+			new:    Object{Rules: Rules{Verbs: "list"}, Other: Other{Foo: "boo"}},
+			update: config.UpdateSetting{Fields: []string{"metadata.name"}, IncludeDiff: true},
+			expectedErrMessage: heredoc.Doc(`
+				while getting diff: 1 error occurred:
+					* while finding value in old obj from jsonpath "metadata.name": metadata is not found`),
 		},
 	}
 	for name, test := range tests {

diff --git a/internal/source/kubernetes/registration.go b/internal/source/kubernetes/registration.go
@@ -300,6 +300,7 @@ func (r registration) qualifyEventForUpdate(
 			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 {
 			r.log.Errorf("while getting diff: %w", err)

diff --git a/test/e2e/bots_test.go b/test/e2e/bots_test.go
@@ -694,7 +694,7 @@ func runBotTest(t *testing.T,
 				},
 			},
 		}
-		err = botDriver.WaitForMessagePostedWithAttachment(botDriver.BotUserID(), botDriver.Channel().ID(), 1, expAttachmentIn)
+		err = botDriver.WaitForMessagePostedWithAttachment(botDriver.BotUserID(), botDriver.Channel().ID(), 2, expAttachmentIn)
 		require.NoError(t, err)
 
 		t.Log("Ensuring bot didn't post anything new in second channel...")
@@ -712,7 +712,7 @@ func runBotTest(t *testing.T,
 
 		t.Log("Ensuring bot didn't post anything new...")
 		time.Sleep(appCfg.Slack.MessageWaitTimeout)
-		err = botDriver.WaitForMessagePostedWithAttachment(botDriver.BotUserID(), botDriver.Channel().ID(), 1, expAttachmentIn)
+		err = botDriver.WaitForMessagePostedWithAttachment(botDriver.BotUserID(), botDriver.Channel().ID(), 2, expAttachmentIn)
 		require.NoError(t, err)
 		err = botDriver.WaitForLastMessageEqual(botDriver.BotUserID(), botDriver.SecondChannel().ID(), expectedMessage)
 		require.NoError(t, err)