Repository: knative/client (go)

Issue:
Add dataBase64 flag for Ping Source
### Feature request

For parity to be able to create a ping source using the kn cli

### Use case
Use the `dataBase64` for binary data as shown in the [docs](https://knative.dev/docs/eventing/samples/ping-source/)

`kn source ping create test-ping-source --schedule "*/2 * * * *" --dataBase64: "ZGF0YQ==" --sink ksvc:event-display`



Pull request:
Adding base64 data handling to ping update command
## Description

Adding base64 data handling to ping update command. Handling comments from #1388 

## Changes

<!-- Please add list of more detailed changes. These changes should be reflected also in the commit messages -->

* Adding a test case to test builder functions
* Handling base64 data in update command
* Modified usage/error strings for more clarity

## Reference

<!-- Please add the corresponding issue number which this pull request is about to fix -->
Fixes #1204 

<!--
Please add an entry to CHANGELOG.adoc file, too, as part of your Pull Request.

In the following cases, add a short description of PR to the unreleased section in CHANGELOG.adoc:

- 🎁 New feature
- 🐛 Bug fix
- ✨ Feature Update
- 🐣 Refactoring
- 🗑️ Remove feature or internal logic

See other entries in CHANGELOG.adoc as an example for how to add the entry, including a reference to the corresponding issue/PR

PLEASE DON'T ADD THAT LINE HERE IN THE PULL-REQUEST DESCRIPTION BUT DIRECTLY IN CHANGELOG.ADOC AND ADD CHANGELOG.ADOC AS PART OF YOUR PULL-REQUEST.
-->

<!--
To automatically lint go code in this pull request uncomment the line below. You get feedback as comments on your pull request then -->

<!--
/lint
-->


Patch:
diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
index 03e18864..72f92524 100644
--- a/CHANGELOG.adoc
+++ b/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]
diff --git a/docs/README.md b/docs/README.md
index 07aada11..8eaf0bea 100644
--- a/docs/README.md
+++ b/docs/README.md
@@ -81,7 +81,7 @@ plugins:
   path-lookup: true
   # Directory from where plugins are looked up. (default: "$base_dir/plugins"
   # where "$base_dir" is the directory where this configuration file is stored)
-  directory: ~/.kn/plugins
+  directory: ~/.config/kn/plugins
 # Eventing related configuration
 eventing:
   # List of sink mappings that allow custom prefixes wherever a sink
diff --git a/docs/cmd/kn_source_ping_create.md b/docs/cmd/kn_source_ping_create.md
index 0e4c079e..6c6e09e8 100644
--- a/docs/cmd/kn_source_ping_create.md
+++ b/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               Data to send in JSON format. This flag can implicitly determine the encoding of the supplied data (text | base64).
+  -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.
diff --git a/docs/cmd/kn_source_ping_update.md b/docs/cmd/kn_source_ping_update.md
index 354523f7..068afd95 100644
--- a/docs/cmd/kn_source_ping_update.md
+++ b/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               Data to send in JSON format. This flag can implicitly determine the encoding of the supplied data (text | base64).
+  -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.
diff --git a/pkg/kn/commands/source/ping/create.go b/pkg/kn/commands/source/ping/create.go
index 6a355db3..e566936a 100644
--- a/pkg/kn/commands/source/ping/create.go
+++ b/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).
diff --git a/pkg/kn/commands/source/ping/describe.go b/pkg/kn/commands/source/ping/describe.go
index 1b2c2899..38bed794 100644
--- a/pkg/kn/commands/source/ping/describe.go
+++ b/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) {
diff --git a/pkg/kn/commands/source/ping/flags.go b/pkg/kn/commands/source/ping/flags.go
index f46005b6..65cd916d 100644
--- a/pkg/kn/commands/source/ping/flags.go
+++ b/pkg/kn/commands/source/ping/flags.go
@@ -15,6 +15,7 @@
 package ping
 
 import (
+	"encoding/base64"
 	"fmt"
 	"sort"
 
@@ -35,15 +36,21 @@ 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", "", fmt.Sprintf("Data to send in JSON format. "+
+		"This flag can implicitly determine the encoding of the supplied data (%s | %s).", textEncoding, base64Encoding))
 
-	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 +167,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
+}
diff --git a/pkg/kn/commands/source/ping/update.go b/pkg/kn/commands/source/ping/update.go
index 7fa139a5..8a3c811c 100644
--- a/pkg/kn/commands/source/ping/update.go
+++ b/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)


Test patch:
diff --git a/pkg/kn/commands/source/ping/describe_test.go b/pkg/kn/commands/source/ping/describe_test.go
index e8744a49..de3653dc 100644
--- a/pkg/kn/commands/source/ping/describe_test.go
+++ b/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()
 }
 
diff --git a/pkg/kn/commands/source/ping/ping_test.go b/pkg/kn/commands/source/ping/ping_test.go
index 5644f78c..ee3af466 100644
--- a/pkg/kn/commands/source/ping/ping_test.go
+++ b/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
diff --git a/pkg/kn/commands/source/ping/update_test.go b/pkg/kn/commands/source/ping/update_test.go
index ccbcbd5d..443000b9 100644
--- a/pkg/kn/commands/source/ping/update_test.go
+++ b/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()
 }
 
diff --git a/test/e2e/ping_test.go b/test/e2e/ping_test.go
index 877349c5..ee0611dc 100644
--- a/test/e2e/ping_test.go
+++ b/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))