Adding base64 data handling to ping update command

CHANGELOG.adoc

@@ -17,6 +17,11 @@
 |===
 | | Description | PR
 
+
+| 🎁
+| Adding base64 data handling to ping update command
+| https://github.com/knative/client/pull/1392[#1392], https://github.com/knative/client/pull/1388[#1388]
+
 | ✨
 | make --cmd flag as an array instead of string
 | https://github.com/knative/client/pull/1380[#1380]

docs/cmd/kn_source_ping_create.md

@@ -18,8 +18,8 @@ kn source ping create NAME --sink SINK
 
 ```
       --ce-override stringArray   Cloud Event overrides to apply before sending event to sink. Example: '--ce-override key=value' You may be provide this flag multiple times. To unset, append "-" to the key (e.g. --ce-override key-).
-  -d, --data string               Json data to send
-  -e, --encoding string           Preferred encoding format. Valid values: text/base64
+  -d, --data string               Json data to send. Can determine if data is plain text or base64 encoded
+  -e, --encoding string           Data encoding format. One of: text|base64
   -h, --help                      help for create
   -n, --namespace string          Specify the namespace to operate in.
       --schedule string           Optional schedule specification in crontab format (e.g. '*/2 * * * *' for every two minutes. By default fire every minute.

docs/cmd/kn_source_ping_update.md

@@ -18,8 +18,8 @@ kn source ping update NAME
 
 ```
       --ce-override stringArray   Cloud Event overrides to apply before sending event to sink. Example: '--ce-override key=value' You may be provide this flag multiple times. To unset, append "-" to the key (e.g. --ce-override key-).
-  -d, --data string               Json data to send
-  -e, --encoding string           Preferred encoding format. Valid values: text/base64
+  -d, --data string               Json data to send. Can determine if data is plain text or base64 encoded
+  -e, --encoding string           Data encoding format. One of: text|base64
   -h, --help                      help for update
   -n, --namespace string          Specify the namespace to operate in.
       --schedule string           Optional schedule specification in crontab format (e.g. '*/2 * * * *' for every two minutes. By default fire every minute.

pkg/kn/commands/source/ping/create.go

@@ -15,7 +15,6 @@
 package ping
 
 import (
-	"encoding/base64"
 	"errors"
 	"fmt"
 
@@ -27,11 +26,6 @@ import (
 	"knative.dev/client/pkg/util"
 )
 
-const (
-	base64Encoding = "base64"
-	textEncoding   = "text"
-)
-
 // NewPingCreateCommand is for creating Ping source COs
 func NewPingCreateCommand(p *commands.KnParams) *cobra.Command {
 	var updateFlags pingUpdateFlags
@@ -76,23 +70,10 @@ func NewPingCreateCommand(p *commands.KnParams) *cobra.Command {
 			}
 			ceOverridesToRemove := util.ParseMinusSuffix(ceOverridesMap)
 
-			var data, dataBase64 string
-
-			switch updateFlags.encoding {
-			case "":
-				if _, err := base64.StdEncoding.DecodeString(updateFlags.data); err == nil {
-					dataBase64 = updateFlags.data
-				} else {
-					data = updateFlags.data
-				}
-			case base64Encoding:
-				dataBase64 = updateFlags.data
-			case textEncoding:
-				data = updateFlags.data
-			default:
-				err := fmt.Errorf("invalid value: %v", updateFlags.encoding)
-				fmt.Fprintf(cmd.OutOrStderr(), "Ping source creation failed with error: %v", err)
-				return err
+			data, dataBase64, err := getDataFields(&updateFlags)
+			if err != nil {
+				return fmt.Errorf("cannot create PingSource %q in namespace "+
+					"%q because: %s", name, namespace, err)
 			}
 
 			err = pingSourceClient.CreatePingSource(cmd.Context(), clientsourcesv1beta2.NewPingSourceBuilder(name).

pkg/kn/commands/source/ping/describe.go

@@ -117,7 +117,11 @@ func NewPingDescribeCommand(p *commands.KnParams) *cobra.Command {
 func writePingSource(dw printers.PrefixWriter, source *clientsourcesv1beta2.PingSource, printDetails bool) {
 	commands.WriteMetadata(dw, &source.ObjectMeta, printDetails)
 	dw.WriteAttribute("Schedule", source.Spec.Schedule)
-	dw.WriteAttribute("Data", source.Spec.Data)
+	if source.Spec.DataBase64 != "" {
+		dw.WriteAttribute("DataBase64", source.Spec.DataBase64)
+	} else {
+		dw.WriteAttribute("Data", source.Spec.Data)
+	}
 }
 
 func writeCeOverrides(dw printers.PrefixWriter, ceOverrides map[string]string) {

pkg/kn/commands/source/ping/describe_test.go

@@ -37,8 +37,16 @@ func TestDescribeRef(t *testing.T) {
 
 	out, err := executePingSourceCommand(pingClient, nil, "describe", "testping")
 	assert.NilError(t, err)
-	assert.Assert(t, util.ContainsAll(out, "*/2 * * * *", "test", "testsvc", "Service", "Overrides", "foo", "bar", "Conditions"))
+	assert.Assert(t, util.ContainsAll(out, "*/2 * * * *", "test", "testsvc", "Service", "Overrides", "foo", "bar", "Conditions", "Data:"))
+	assert.Assert(t, util.ContainsNone(out, "DataBase64"))
+	pingRecorder.Validate()
 
+	pingRecorder.GetPingSource("testping",
+		createPingSource("testping", "*/2 * * * *", "", "cGluZw==", "testsvc", map[string]string{"foo": "bar"}), nil)
+	out, err = executePingSourceCommand(pingClient, nil, "describe", "testping")
+	assert.NilError(t, err)
+	assert.Assert(t, util.ContainsAll(out, "*/2 * * * *", "test", "testsvc", "Service", "Overrides", "foo", "bar", "Conditions", "DataBase64"))
+	assert.Assert(t, util.ContainsNone(out, "Data:"))
 	pingRecorder.Validate()
 }
 

pkg/kn/commands/source/ping/flags.go

@@ -15,6 +15,7 @@
 package ping
 
 import (
+	"encoding/base64"
 	"fmt"
 	"sort"
 
@@ -35,15 +36,20 @@ type pingUpdateFlags struct {
 	ceOverrides []string
 }
 
+const (
+	base64Encoding = "base64"
+	textEncoding   = "text"
+)
+
 func (c *pingUpdateFlags) addFlags(cmd *cobra.Command) {
 	cmd.Flags().StringVar(&c.schedule,
 		"schedule",
 		"",
 		"Optional schedule specification in crontab format (e.g. '*/2 * * * *' for every two minutes. By default fire every minute.")
 
-	cmd.Flags().StringVarP(&c.data, "data", "d", "", "Json data to send")
+	cmd.Flags().StringVarP(&c.data, "data", "d", "", "Json data to send. Can determine if data is plain text or base64 encoded")
 
-	cmd.Flags().StringVarP(&c.encoding, "encoding", "e", "", "Preferred encoding format. Valid values: text/base64")
+	cmd.Flags().StringVarP(&c.encoding, "encoding", "e", "", fmt.Sprintf("Data encoding format. One of: %s|%s", textEncoding, base64Encoding))
 
 	cmd.Flags().StringArrayVar(&c.ceOverrides,
 		"ce-override",
@@ -160,3 +166,23 @@ func printSourceListWithNamespace(sourceList *sourcesv1beta2.PingSourceList, opt
 
 	return append(rows, others...), nil
 }
+
+func getDataFields(updateFlags *pingUpdateFlags) (string, string, error) {
+	var data, dataBase64 string
+	var err error
+	switch updateFlags.encoding {
+	case "":
+		if _, err := base64.StdEncoding.DecodeString(updateFlags.data); err == nil {
+			dataBase64 = updateFlags.data
+		} else {
+			data = updateFlags.data
+		}
+	case base64Encoding:
+		dataBase64 = updateFlags.data
+	case textEncoding:
+		data = updateFlags.data
+	default:
+		err = fmt.Errorf("invalid value: %s. Accepted values are: %s|%s", updateFlags.encoding, textEncoding, base64Encoding)
+	}
+	return data, dataBase64, err
+}

pkg/kn/commands/source/ping/ping_test.go

@@ -16,6 +16,9 @@ package ping
 
 import (
 	"bytes"
+	"testing"
+
+	"gotest.tools/v3/assert"
 
 	"k8s.io/client-go/tools/clientcmd"
 	sourcesv1beta2 "knative.dev/eventing/pkg/apis/sources/v1beta2"
@@ -53,6 +56,22 @@ current-context: x
 	}
 }
 
+func TestPingBuilder(t *testing.T) {
+	name := "mockName"
+	schedule := "* * * * *"
+	data := "mockData"
+	dataBase64 := "mockDataBase64"
+	sink := "mockService"
+	ceOverrideMap := map[string]string{}
+	ps := createPingSource(name, schedule, data, dataBase64, sink, ceOverrideMap)
+	assert.Equal(t, name, ps.Name)
+	assert.Equal(t, schedule, ps.Spec.Schedule)
+	assert.Equal(t, data, ps.Spec.Data)
+	assert.Equal(t, dataBase64, ps.Spec.DataBase64)
+	assert.Equal(t, sink, ps.Spec.Sink.Ref.Name)
+	assert.DeepEqual(t, ceOverrideMap, ps.Spec.CloudEventOverrides.Extensions)
+}
+
 func executePingSourceCommand(pingSourceClient clientv1beta2.KnPingSourcesClient, dynamicClient kndynamic.KnDynamicClient, args ...string) (string, error) {
 	knParams := &commands.KnParams{}
 	knParams.ClientConfig = blankConfig

pkg/kn/commands/source/ping/update.go

@@ -70,8 +70,15 @@ func NewPingUpdateCommand(p *commands.KnParams) *cobra.Command {
 			if cmd.Flags().Changed("schedule") {
 				b.Schedule(updateFlags.schedule)
 			}
+
+			data, dataBase64, err := getDataFields(&updateFlags)
+			if err != nil {
+				return fmt.Errorf("cannot update PingSource %q in namespace "+
+					"%q because: %s", name, namespace, err)
+			}
+
 			if cmd.Flags().Changed("data") {
-				b.Data(updateFlags.data)
+				b.Data(data).DataBase64(dataBase64)
 			}
 			if cmd.Flags().Changed("sink") {
 				destination, err := sinkFlags.ResolveSink(cmd.Context(), dynamicClient, namespace)

pkg/kn/commands/source/ping/update_test.go

@@ -43,6 +43,28 @@ func TestSimplePingUpdate(t *testing.T) {
 	assert.NilError(t, err)
 	assert.Assert(t, util.ContainsAll(out, "updated", "default", "testsource"))
 
+	pingRecorder.GetPingSource("testsource", createPingSource("testsource", "* * * * */1", "maxwell", "", "mysvc", nil), nil)
+	pingRecorder.UpdatePingSource(createPingSource("testsource", "* * * * */3", "", "hello", "mysvc", nil), nil)
+	out, err = executePingSourceCommand(pingSourceClient, dynamicClient, "update", "--schedule", "* * * * */3", "testsource", "--data", "hello", "--encoding", "base64")
+	assert.NilError(t, err)
+	assert.Assert(t, util.ContainsAll(out, "updated", "default", "testsource"))
+
+	pingRecorder.GetPingSource("testsource", createPingSource("testsource", "* * * * */1", "maxwell", "", "mysvc", nil), nil)
+	pingRecorder.UpdatePingSource(createPingSource("testsource", "* * * * */3", "hello", "", "mysvc", nil), nil)
+	out, err = executePingSourceCommand(pingSourceClient, dynamicClient, "update", "--schedule", "* * * * */3", "testsource", "--data", "hello", "--encoding", "text")
+	assert.NilError(t, err)
+	assert.Assert(t, util.ContainsAll(out, "updated", "default", "testsource"))
+
+	pingRecorder.GetPingSource("testsource", createPingSource("testsource", "* * * * */1", "maxwell", "", "mysvc", nil), nil)
+	pingRecorder.UpdatePingSource(createPingSource("testsource", "* * * * */3", "", "aGVsbG8=", "mysvc", nil), nil)
+	out, err = executePingSourceCommand(pingSourceClient, dynamicClient, "update", "--schedule", "* * * * */3", "testsource", "--data", "aGVsbG8=")
+	assert.NilError(t, err)
+	assert.Assert(t, util.ContainsAll(out, "updated", "default", "testsource"))
+
+	pingRecorder.GetPingSource("testsource", createPingSource("testsource", "* * * * */1", "maxwell", "", "mysvc", nil), nil)
+	_, err = executePingSourceCommand(pingSourceClient, dynamicClient, "update", "--schedule", "* * * * */3", "testsource", "--data", "aGVsbG8=", "--encoding", "mockencoding")
+	assert.ErrorContains(t, err, "invalid value")
+
 	pingRecorder.Validate()
 }
 

test/e2e/ping_test.go

@@ -64,6 +64,21 @@ func TestSourcePing(t *testing.T) {
 	mymsg := "This is a message from Ping."
 	pingSourceCreate(r, "testpingsource3", "*/1 * * * *", mymsg, "ksvc:testsvc1")
 	verifyPingSourceDescribe(r, "testpingsource3", "*/1 * * * *", mymsg, "testsvc1")
+
+	t.Log("create Ping source with base64 data without encoding flag")
+	pingSourceCreate(r, "testpingsource4", "* * * * */1", "cGluZw==", "ksvc:testsvc1")
+	verifyPingSourceDescribe(r, "testpingsource4", "* * * * */1", "cGluZw==", "testsvc1")
+
+	t.Log("create Ping source with text data and encoding flag")
+	pingSourceCreateWithEncoding(r, "testpingsource6", "* * * * */1", "cGluZw==", "ksvc:testsvc1", "text")
+	verifyPingSourceDescribe(r, "testpingsource6", "* * * * */1", "cGluZw==", "testsvc1")
+
+	t.Log("update Ping source with base64 data and encoding flag")
+	pingSourceUpdateData(r, "testpingsource6", "cGluZw==", "base64")
+	verifyPingSourceDescribe(r, "testpingsource6", "* * * * */1", "cGluZw==", "testsvc1")
+
+	t.Log("update Ping source with base64 data and encoding flag with error")
+	pingSourceUpdateData(r, "testpingsource6", "cGluZw==", "base65")
 }
 
 func pingSourceCreate(r *test.KnRunResultCollector, sourceName string, schedule string, data string, sink string) {
@@ -73,6 +88,13 @@ func pingSourceCreate(r *test.KnRunResultCollector, sourceName string, schedule
 	r.AssertNoError(out)
 }
 
+func pingSourceCreateWithEncoding(r *test.KnRunResultCollector, sourceName string, schedule string, data string, sink string, encoding string) {
+	out := r.KnTest().Kn().Run("source", "ping", "create", sourceName,
+		"--schedule", schedule, "--data", data, "--sink", sink, "--encoding", encoding)
+	assert.Check(r.T(), util.ContainsAllIgnoreCase(out.Stdout, "ping", "source", sourceName, "created", "namespace", r.KnTest().Kn().Namespace()))
+	r.AssertNoError(out)
+}
+
 func pingSourceDelete(r *test.KnRunResultCollector, sourceName string) {
 	out := r.KnTest().Kn().Run("source", "ping", "delete", sourceName)
 	assert.Check(r.T(), util.ContainsAllIgnoreCase(out.Stdout, "ping", "source", sourceName, "deleted", "namespace", r.KnTest().Kn().Namespace()))
@@ -98,6 +120,16 @@ func pingSourceUpdateSink(r *test.KnRunResultCollector, sourceName string, sink
 	r.AssertNoError(out)
 }
 
+func pingSourceUpdateData(r *test.KnRunResultCollector, sourceName string, data string, encoding string) {
+	out := r.KnTest().Kn().Run("source", "ping", "update", sourceName, "--data", data, "--encoding", encoding)
+	if encoding != "text" && encoding != "base64" {
+		assert.Check(r.T(), util.ContainsAll(out.Stderr, "cannot update PingSource", "invalid value"))
+		return
+	}
+	assert.Check(r.T(), util.ContainsAll(out.Stdout, sourceName, "updated", "namespace", r.KnTest().Kn().Namespace()))
+	r.AssertNoError(out)
+}
+
 func verifyPingSourceDescribe(r *test.KnRunResultCollector, sourceName string, schedule string, data string, sink string) {
 	out := r.KnTest().Kn().Run("source", "ping", "describe", sourceName)
 	assert.Check(r.T(), util.ContainsAllIgnoreCase(out.Stdout, sourceName, schedule, data, sink))