add more tests and update based on comments

docs/cmd/kn_source_apiserver_create.md

@@ -15,7 +15,7 @@ kn source apiserver create NAME --resource RESOURCE --service-account ACCOUNTNAM
 ```
 
   # Create an ApiServerSource 'k8sevents' which consumes Kubernetes events and sends message to service 'mysvc' as a cloudevent
-  kn source apiserver create k8sevents --resource Event --service-account myaccountname --sink svc:mysvc
+  kn source apiserver create k8sevents --resource Event:v1 --service-account myaccountname --sink svc:mysvc
 ```
 
 ### Options
@@ -26,8 +26,8 @@ kn source apiserver create NAME --resource RESOURCE --service-account ACCOUNTNAM
                                  "Ref" sends only the reference to the resource,
                                  "Resource" send the full resource. (default "Ref")
   -n, --namespace string         Specify the namespace to operate in.
-      --resource stringArray     Specification for which events to listen, in the format Kind:APIVersion:isController, e.g. Deployment:apps/v1:true.
-                                 "isController" can be omitted and is "false" by default.
+      --resource stringArray     Specification for which events to listen, in the format Kind:APIVersion:isController, e.g. "Event:v1:true".
+                                 "isController" can be omitted and is "false" by default, e.g. "Event:v1".
       --service-account string   Name of the service account to use to run this source
   -s, --sink string              Addressable sink for events
 ```

docs/cmd/kn_source_apiserver_update.md

@@ -26,8 +26,8 @@ kn source apiserver update NAME --resource RESOURCE --service-account ACCOUNTNAM
                                  "Ref" sends only the reference to the resource,
                                  "Resource" send the full resource. (default "Ref")
   -n, --namespace string         Specify the namespace to operate in.
-      --resource stringArray     Specification for which events to listen, in the format Kind:APIVersion:isController, e.g. Deployment:apps/v1:true.
-                                 "isController" can be omitted and is "false" by default.
+      --resource stringArray     Specification for which events to listen, in the format Kind:APIVersion:isController, e.g. "Event:v1:true".
+                                 "isController" can be omitted and is "false" by default, e.g. "Event:v1".
       --service-account string   Name of the service account to use to run this source
   -s, --sink string              Addressable sink for events
 ```

pkg/eventing/sources/v1alpha1/apiserver_client.go

@@ -15,9 +15,6 @@
 package v1alpha1
 
 import (
-	"fmt"
-	"reflect"
-
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kn_errors "knative.dev/client/pkg/errors"
 	"knative.dev/eventing/pkg/apis/sources/v1alpha1"
@@ -159,43 +156,6 @@ func (b *APIServerSourceBuilder) Resources(resources []v1alpha1.ApiServerResourc
 	return b
 }
 
-// AddResource which should be streamed
-func (b *APIServerSourceBuilder) AddResource(version string, kind string, isController bool) *APIServerSourceBuilder {
-	resources := b.apiServerSource.Spec.Resources
-	if resources == nil {
-		resources = []v1alpha1.ApiServerResource{}
-		b.apiServerSource.Spec.Resources = resources
-	}
-	resourceRef := v1alpha1.ApiServerResource{
-		APIVersion: version,
-		Kind:       kind,
-		Controller: isController,
-	}
-	b.apiServerSource.Spec.Resources = append(resources, resourceRef)
-	return b
-}
-
-// RemoveResource which should be streamed
-func (b *APIServerSourceBuilder) RemoveResource(version string, kind string, isController bool) (*APIServerSourceBuilder, error) {
-	resources := b.apiServerSource.Spec.Resources
-	if resources == nil {
-		resources = []v1alpha1.ApiServerResource{}
-		b.apiServerSource.Spec.Resources = resources
-	}
-	resourceRef := v1alpha1.ApiServerResource{
-		APIVersion: version,
-		Kind:       kind,
-		Controller: isController,
-	}
-	for i, k := range resources {
-		if reflect.DeepEqual(k, resourceRef) {
-			resources = append(resources[:i], resources[i+1:]...)
-			return b, nil
-		}
-	}
-	return b, fmt.Errorf("cannot find resource %s:%s:%t to remove", version, kind, isController)
-}
-
 // ServiceAccount with which this source should operate
 func (b *APIServerSourceBuilder) ServiceAccount(sa string) *APIServerSourceBuilder {
 	b.apiServerSource.Spec.ServiceAccountName = sa

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

@@ -35,7 +35,7 @@ func NewAPIServerCreateCommand(p *commands.KnParams) *cobra.Command {
 		Short: "Create an ApiServer source.",
 		Example: `
   # Create an ApiServerSource 'k8sevents' which consumes Kubernetes events and sends message to service 'mysvc' as a cloudevent
-  kn source apiserver create k8sevents --resource Event --service-account myaccountname --sink svc:mysvc`,
+  kn source apiserver create k8sevents --resource Event:v1 --service-account myaccountname --sink svc:mysvc`,
 
 		RunE: func(cmd *cobra.Command, args []string) (err error) {
 			if len(args) != 1 {
@@ -71,9 +71,7 @@ func NewAPIServerCreateCommand(p *commands.KnParams) *cobra.Command {
 			if err != nil {
 				return err
 			}
-			for _, k := range resources {
-				b.AddResource(k.ApiVersion, k.Kind, k.IsController)
-			}
+			b.Resources(resources)
 
 			err = apiSourceClient.CreateAPIServerSource(b.Build())
 

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

@@ -16,6 +16,7 @@ package apiserver
 
 import (
 	"fmt"
+	"reflect"
 	"sort"
 	"strconv"
 	"strings"
@@ -41,15 +42,9 @@ type APIServerSourceUpdateFlags struct {
 	Resources          []string
 }
 
-type resourceSpec struct {
-	Kind         string
-	ApiVersion   string
-	IsController bool
-}
-
-// getAPIServerResourceArray is to return an array of ApiServerResource from a string. A sample is Event:v1:true,Pod:v2:false
-func (f *APIServerSourceUpdateFlags) getAPIServerResourceArray() ([]resourceSpec, error) {
-	var resourceList []resourceSpec
+// getAPIServerResourceArray is to construct an array of resources.
+func (f *APIServerSourceUpdateFlags) getAPIServerResourceArray() ([]v1alpha1.ApiServerResource, error) {
+	var resourceList []v1alpha1.ApiServerResource
 	for _, r := range f.Resources {
 		resourceSpec, err := getValidResource(r)
 		if err != nil {
@@ -60,46 +55,88 @@ func (f *APIServerSourceUpdateFlags) getAPIServerResourceArray() ([]resourceSpec
 	return resourceList, nil
 }
 
-// getAPIServerResourceArray is to return an array of ApiServerResource from a string. A sample is Event:v1:true
-func (f *APIServerSourceUpdateFlags) getUpdateAPIServerResourceArray() ([]resourceSpec, []resourceSpec, error) {
-	var added []resourceSpec
-	var removed []resourceSpec
+// updateExistingAPIServerResourceArray is to update an array of resources.
+func (f *APIServerSourceUpdateFlags) updateExistingAPIServerResourceArray(existing []v1alpha1.ApiServerResource) ([]v1alpha1.ApiServerResource, error) {
+	var found bool
 
-	addedArray, removedArray := util.AddListAndRemovalListFromArray(f.Resources)
-	for _, r := range addedArray {
-		resourceSpec, err := getValidResource(r)
-		if err != nil {
-			return nil, nil, err
+	added, removed, err := f.getUpdateAPIServerResourceArray()
+	if err != nil {
+		return nil, err
+	}
+
+	if existing == nil {
+		existing = []v1alpha1.ApiServerResource{}
+	}
+
+	existing = append(existing, added...)
+
+	for _, item := range removed {
+		found = false
+		for i, ref := range existing {
+			if reflect.DeepEqual(item, ref) {
+				existing = append(existing[:i], existing[i+1:]...)
+				found = true
+				break
+			}
+		}
+		if !found {
+			return nil, fmt.Errorf("cannot find resource %s:%s:%t to remove", item.Kind, item.APIVersion, item.Controller)
 		}
-		added = append(added, *resourceSpec)
 	}
-	for _, r := range removedArray {
+	return existing, nil
+}
+
+// getUpdateAPIServerResourceArray is to construct an array of resources for update action.
+func (f *APIServerSourceUpdateFlags) getUpdateAPIServerResourceArray() ([]v1alpha1.ApiServerResource, []v1alpha1.ApiServerResource, error) {
+	addedArray, removedArray := util.AddedAndRemovalListsFromArray(f.Resources)
+	added, err := constructApiServerResourceArray(addedArray)
+	if err != nil {
+		return nil, nil, err
+	}
+	removed, err := constructApiServerResourceArray(removedArray)
+	if err != nil {
+		return nil, nil, err
+	}
+	return added, removed, nil
+}
+
+func constructApiServerResourceArray(s []string) ([]v1alpha1.ApiServerResource, error) {
+	array := make([]v1alpha1.ApiServerResource, 0)
+	for _, r := range s {
 		resourceSpec, err := getValidResource(r)
 		if err != nil {
-			return nil, nil, err
+			return array, err
 		}
-		removed = append(removed, *resourceSpec)
+		array = append(array, *resourceSpec)
 	}
-	return added, removed, nil
+	return array, nil
 }
 
-func getValidResource(resource string) (*resourceSpec, error) {
-	var isController = false //false as default
+//getValidResource is to parse resource spec from a string
+func getValidResource(resource string) (*v1alpha1.ApiServerResource, error) {
+	var isController bool
 	var err error
 
-	parts := strings.Split(resource, apiVersionSplitChar)
+	parts := strings.SplitN(resource, apiVersionSplitChar, 3)
+	if len(parts[0]) == 0 {
+		return nil, fmt.Errorf("cannot find 'Kind' part in resource specification %s (expected: <Kind:ApiVersion[:controllerFlag]>", resource)
+	}
 	kind := parts[0]
-	if len(parts) < 2 {
-		return nil, fmt.Errorf("no APIVersion given for resource %s", resource)
+
+	if len(parts) < 2 || len(parts[1]) == 0 {
+		return nil, fmt.Errorf("cannot find 'APIVersion' part in resource specification %s (expected: <Kind:ApiVersion[:controllerFlag]>", resource)
 	}
 	version := parts[1]
-	if len(parts) >= 3 {
+
+	if len(parts) >= 3 && len(parts[2]) > 0 {
 		isController, err = strconv.ParseBool(parts[2])
 		if err != nil {
-			return nil, fmt.Errorf("cannot parse controller flage in resource specification %s", resource)
+			return nil, fmt.Errorf("controller flag is not a boolean in resource specification %s (expected: <Kind:ApiVersion[:controllerFlag]>)", resource)
 		}
+	} else {
+		isController = false
 	}
-	return &resourceSpec{Kind: kind, ApiVersion: version, IsController: isController}, nil
+	return &v1alpha1.ApiServerResource{Kind: kind, APIVersion: version, Controller: isController}, nil
 }
 
 //Add is to set parameters
@@ -117,8 +154,8 @@ func (f *APIServerSourceUpdateFlags) Add(cmd *cobra.Command) {
 	cmd.Flags().StringArrayVar(&f.Resources,
 		"resource",
 		[]string{},
-		`Specification for which events to listen, in the format Kind:APIVersion:isController, e.g. Deployment:apps/v1:true.
-"isController" can be omitted and is "false" by default.`)
+		`Specification for which events to listen, in the format Kind:APIVersion:isController, e.g. "Event:v1:true".
+"isController" can be omitted and is "false" by default, e.g. "Event:v1".`)
 }
 
 // APIServerSourceListHandlers handles printing human readable table for `kn source apiserver list` command's output