diff --git a/docs/cmd/kn_source_apiserver_create.md b/docs/cmd/kn_source_apiserver_create.md index 3ff858d7b9..4483d5f244 100644 --- a/docs/cmd/kn_source_apiserver_create.md +++ b/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 strings 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 ``` diff --git a/docs/cmd/kn_source_apiserver_update.md b/docs/cmd/kn_source_apiserver_update.md index 4b5a816c88..f6c561685c 100644 --- a/docs/cmd/kn_source_apiserver_update.md +++ b/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 strings 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 ``` diff --git a/pkg/kn/commands/source/apiserver/create.go b/pkg/kn/commands/source/apiserver/create.go index 2aa483ac4d..f538d38d76 100644 --- a/pkg/kn/commands/source/apiserver/create.go +++ b/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 { @@ -62,19 +62,18 @@ func NewAPIServerCreateCommand(p *commands.KnParams) *cobra.Command { "because: %s", name, namespace, err) } - // create - resources, err := updateFlags.GetAPIServerResourceArray() + b := v1alpha1.NewAPIServerSourceBuilder(name). + ServiceAccount(updateFlags.ServiceAccountName). + Mode(updateFlags.Mode). + Sink(objectRef) + + resources, err := updateFlags.getAPIServerResourceArray() if err != nil { return err } + b.Resources(resources) - err = apiSourceClient.CreateAPIServerSource( - v1alpha1.NewAPIServerSourceBuilder(name). - ServiceAccount(updateFlags.ServiceAccountName). - Mode(updateFlags.Mode). - Resources(*resources). - Sink(objectRef). - Build()) + err = apiSourceClient.CreateAPIServerSource(b.Build()) if err != nil { return fmt.Errorf( diff --git a/pkg/kn/commands/source/apiserver/create_flag_test.go b/pkg/kn/commands/source/apiserver/create_flag_test.go deleted file mode 100644 index 07450d81d3..0000000000 --- a/pkg/kn/commands/source/apiserver/create_flag_test.go +++ /dev/null @@ -1,105 +0,0 @@ -// Copyright © 2019 The Knative Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package apiserver - -import ( - "testing" - - "gotest.tools/assert" - sources_v1alpha1 "knative.dev/eventing/pkg/apis/legacysources/v1alpha1" -) - -func TestGetAPIServerResourceArray(t *testing.T) { - t.Run("get single apiserver resource", func(t *testing.T) { - createFlag := APIServerSourceUpdateFlags{ - ServiceAccountName: "test-sa", - Mode: "Ref", - Resources: []string{"Service:serving.knative.dev/v1alpha1:true"}, - } - created, _ := createFlag.GetAPIServerResourceArray() - wanted := &[]sources_v1alpha1.ApiServerResource{{ - APIVersion: "serving.knative.dev/v1alpha1", - Kind: "Service", - Controller: true, - }} - assert.DeepEqual(t, wanted, created) - - // default isController - createFlag = APIServerSourceUpdateFlags{ - ServiceAccountName: "test-sa", - Mode: "Ref", - Resources: []string{"Service:serving.knative.dev/v1alpha1"}, - } - created, _ = createFlag.GetAPIServerResourceArray() - wanted = &[]sources_v1alpha1.ApiServerResource{{ - APIVersion: "serving.knative.dev/v1alpha1", - Kind: "Service", - Controller: false, - }} - assert.DeepEqual(t, wanted, created) - - // default api version and isController - createFlag = APIServerSourceUpdateFlags{ - ServiceAccountName: "test-sa", - Mode: "Ref", - Resources: []string{"Service:v1"}, - } - created, _ = createFlag.GetAPIServerResourceArray() - wanted = &[]sources_v1alpha1.ApiServerResource{{ - APIVersion: "v1", - Kind: "Service", - Controller: false, - }} - assert.DeepEqual(t, wanted, created) - }) - - t.Run("get multiple apiserver resources", func(t *testing.T) { - createFlag := APIServerSourceUpdateFlags{ - ServiceAccountName: "test-sa", - Mode: "Resource", - Resources: []string{"Event:v1:true", "Pod:v2:false"}, - } - created, _ := createFlag.GetAPIServerResourceArray() - wanted := &[]sources_v1alpha1.ApiServerResource{{ - APIVersion: "v1", - Kind: "Event", - Controller: true, - }, { - APIVersion: "v2", - Kind: "Pod", - Controller: false, - }} - assert.DeepEqual(t, wanted, created) - - // default api version and isController - createFlag = APIServerSourceUpdateFlags{ - ServiceAccountName: "test-sa", - Mode: "Resource", - Resources: []string{"Event:v1", "Pod:v1"}, - } - created, _ = createFlag.GetAPIServerResourceArray() - - wanted = &[]sources_v1alpha1.ApiServerResource{{ - APIVersion: "v1", - Kind: "Event", - Controller: false, - }, { - APIVersion: "v1", - Kind: "Pod", - Controller: false, - }} - assert.DeepEqual(t, wanted, created) - }) -} diff --git a/pkg/kn/commands/source/apiserver/flags.go b/pkg/kn/commands/source/apiserver/flags.go index bdce1f964e..2ba9d632e7 100644 --- a/pkg/kn/commands/source/apiserver/flags.go +++ b/pkg/kn/commands/source/apiserver/flags.go @@ -16,6 +16,7 @@ package apiserver import ( "fmt" + "reflect" "sort" "strconv" "strings" @@ -27,6 +28,7 @@ import ( metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1" hprinters "knative.dev/client/pkg/printers" + "knative.dev/client/pkg/util" ) const ( @@ -40,47 +42,101 @@ 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() (*[]v1alpha1.ApiServerResource, error) { +// 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 { return nil, err } - resourceRef := v1alpha1.ApiServerResource{ - APIVersion: resourceSpec.apiVersion, - Kind: resourceSpec.kind, - Controller: resourceSpec.isController, + resourceList = append(resourceList, *resourceSpec) + } + return resourceList, nil +} + +// updateExistingAPIServerResourceArray is to update an array of resources. +func (f *APIServerSourceUpdateFlags) updateExistingAPIServerResourceArray(existing []v1alpha1.ApiServerResource) ([]v1alpha1.ApiServerResource, error) { + var found bool + + 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) } - resourceList = append(resourceList, resourceRef) } - return &resourceList, nil + return existing, nil } -func getValidResource(resource string) (*resourceSpec, error) { - var isController = false //false as default +// 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 array, err + } + array = append(array, *resourceSpec) + } + return array, nil +} + +//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: ", 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: ", 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: )", resource) } + } else { + isController = false } - return &resourceSpec{apiVersion: version, kind: kind, isController: isController}, nil + return &v1alpha1.ApiServerResource{Kind: kind, APIVersion: version, Controller: isController}, nil } //Add is to set parameters @@ -95,11 +151,11 @@ func (f *APIServerSourceUpdateFlags) Add(cmd *cobra.Command) { `The mode the receive adapter controller runs under:, "Ref" sends only the reference to the resource, "Resource" send the full resource.`) - cmd.Flags().StringSliceVar(&f.Resources, + cmd.Flags().StringArrayVar(&f.Resources, "resource", - nil, - `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.`) + []string{}, + `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 diff --git a/pkg/kn/commands/source/apiserver/flags_test.go b/pkg/kn/commands/source/apiserver/flags_test.go new file mode 100644 index 0000000000..6b8ceda16a --- /dev/null +++ b/pkg/kn/commands/source/apiserver/flags_test.go @@ -0,0 +1,212 @@ +// Copyright © 2019 The Knative Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package apiserver + +import ( + "testing" + + "gotest.tools/assert" + "knative.dev/eventing/pkg/apis/legacysources/v1alpha1" +) + +func TestGetAPIServerResourceArray(t *testing.T) { + t.Run("get single apiserver resource", func(t *testing.T) { + createFlag := APIServerSourceUpdateFlags{ + ServiceAccountName: "test-sa", + Mode: "Ref", + Resources: []string{"Service:serving.knative.dev/v1alpha1:true"}, + } + created, _ := createFlag.getAPIServerResourceArray() + wanted := []v1alpha1.ApiServerResource{{ + Kind: "Service", + APIVersion: "serving.knative.dev/v1alpha1", + Controller: true, + }} + assert.DeepEqual(t, wanted, created) + }) + + t.Run("get single apiserver resource when isController is default", func(t *testing.T) { + createFlag := APIServerSourceUpdateFlags{ + ServiceAccountName: "test-sa", + Mode: "Ref", + Resources: []string{"Service:serving.knative.dev/v1alpha1"}, + } + created, _ := createFlag.getAPIServerResourceArray() + wanted := []v1alpha1.ApiServerResource{{ + Kind: "Service", + APIVersion: "serving.knative.dev/v1alpha1", + Controller: false, + }} + assert.DeepEqual(t, wanted, created) + }) + + t.Run("get multiple apiserver resources", func(t *testing.T) { + createFlag := APIServerSourceUpdateFlags{ + ServiceAccountName: "test-sa", + Mode: "Resource", + Resources: []string{"Event:v1:true", "Pod:v2:false"}, + } + created, _ := createFlag.getAPIServerResourceArray() + wanted := []v1alpha1.ApiServerResource{{ + Kind: "Event", + APIVersion: "v1", + Controller: true, + }, { + Kind: "Pod", + APIVersion: "v2", + Controller: false, + }} + assert.DeepEqual(t, wanted, created) + }) + + t.Run("get multiple apiserver resources when isController is default", func(t *testing.T) { + createFlag := APIServerSourceUpdateFlags{ + ServiceAccountName: "test-sa", + Mode: "Resource", + Resources: []string{"Event:v1", "Pod:v1"}, + } + created, _ := createFlag.getAPIServerResourceArray() + + wanted := []v1alpha1.ApiServerResource{{ + Kind: "Event", + APIVersion: "v1", + Controller: false, + }, { + Kind: "Pod", + APIVersion: "v1", + Controller: false, + }} + assert.DeepEqual(t, wanted, created) + }) + + t.Run("get apiserver resource when isController has error", func(t *testing.T) { + createFlag := APIServerSourceUpdateFlags{ + ServiceAccountName: "test-sa", + Mode: "Ref", + Resources: []string{"Event:v1:xxx"}, + } + _, err := createFlag.getAPIServerResourceArray() + errorMsg := "controller flag is not a boolean in resource specification Event:v1:xxx (expected: )" + assert.Error(t, err, errorMsg) + }) + + t.Run("get apiserver resources when kind has error", func(t *testing.T) { + createFlag := APIServerSourceUpdateFlags{ + ServiceAccountName: "test-sa", + Mode: "Ref", + Resources: []string{":v2:true"}, + } + _, err := createFlag.getAPIServerResourceArray() + errorMsg := "cannot find 'Kind' part in resource specification :v2:true (expected: " + assert.Error(t, err, errorMsg) + }) + + t.Run("get apiserver resources when APIVersion has error", func(t *testing.T) { + createFlag := APIServerSourceUpdateFlags{ + ServiceAccountName: "test-sa", + Mode: "Ref", + Resources: []string{"kind"}, + } + _, err := createFlag.getAPIServerResourceArray() + errorMsg := "cannot find 'APIVersion' part in resource specification kind (expected: " + assert.Error(t, err, errorMsg) + }) +} + +func TestGetUpdateAPIServerResourceArray(t *testing.T) { + t.Run("get removed apiserver resources", func(t *testing.T) { + createFlag := APIServerSourceUpdateFlags{ + ServiceAccountName: "test-sa", + Mode: "Resource", + Resources: []string{"Event:v1:true", "Pod:v2:false-"}, + } + added, removed, _ := createFlag.getUpdateAPIServerResourceArray() + addwanted := []v1alpha1.ApiServerResource{{ + Kind: "Event", + APIVersion: "v1", + Controller: true, + }} + removewanted := []v1alpha1.ApiServerResource{{ + Kind: "Pod", + APIVersion: "v2", + Controller: false, + }} + assert.DeepEqual(t, added, addwanted) + assert.DeepEqual(t, removed, removewanted) + + // default api version and isController + createFlag = APIServerSourceUpdateFlags{ + ServiceAccountName: "test-sa", + Mode: "Resource", + Resources: []string{"Event:v1-", "Pod:v1"}, + } + added, removed, _ = createFlag.getUpdateAPIServerResourceArray() + + removewanted = []v1alpha1.ApiServerResource{{ + Kind: "Event", + APIVersion: "v1", + Controller: false, + }} + addwanted = []v1alpha1.ApiServerResource{{ + Kind: "Pod", + APIVersion: "v1", + Controller: false, + }} + assert.DeepEqual(t, added, addwanted) + assert.DeepEqual(t, removed, removewanted) + }) +} + +func TestUpdateExistingAPIServerResourceArray(t *testing.T) { + existing := []v1alpha1.ApiServerResource{{ + Kind: "Event", + APIVersion: "v1", + Controller: false, + }, { + Kind: "Pod", + APIVersion: "v1", + Controller: false, + }} + + t.Run("update existing apiserver resources", func(t *testing.T) { + createFlag := APIServerSourceUpdateFlags{ + ServiceAccountName: "test-sa", + Mode: "Resource", + Resources: []string{"Deployment:v1:true", "Pod:v1:false-"}, + } + updated, _ := createFlag.updateExistingAPIServerResourceArray(existing) + updatedWanted := []v1alpha1.ApiServerResource{{ + Kind: "Event", + APIVersion: "v1", + Controller: false, + }, { + Kind: "Deployment", + APIVersion: "v1", + Controller: true, + }} + assert.DeepEqual(t, updated, updatedWanted) + }) + + t.Run("update existing apiserver resources with error", func(t *testing.T) { + createFlag := APIServerSourceUpdateFlags{ + ServiceAccountName: "test-sa", + Mode: "Resource", + Resources: []string{"Deployment:v1:true", "Pod:v2:false-"}, + } + _, err := createFlag.updateExistingAPIServerResourceArray(existing) + errorMsg := "cannot find resource Pod:v2:false to remove" + assert.Error(t, err, errorMsg) + }) +} diff --git a/pkg/kn/commands/source/apiserver/update.go b/pkg/kn/commands/source/apiserver/update.go index f97a3d04d4..820733733d 100644 --- a/pkg/kn/commands/source/apiserver/update.go +++ b/pkg/kn/commands/source/apiserver/update.go @@ -74,11 +74,11 @@ func NewAPIServerUpdateCommand(p *commands.KnParams) *cobra.Command { } if cmd.Flags().Changed("resource") { - resources, err := apiServerUpdateFlags.GetAPIServerResourceArray() + updateExisting, err := apiServerUpdateFlags.updateExistingAPIServerResourceArray(source.Spec.Resources) if err != nil { return err } - b.Resources(*resources) + b.Resources(updateExisting) } if cmd.Flags().Changed("sink") { diff --git a/pkg/util/parsing_helper.go b/pkg/util/parsing_helper.go index eaf070bc95..0837a4f87e 100644 --- a/pkg/util/parsing_helper.go +++ b/pkg/util/parsing_helper.go @@ -90,6 +90,20 @@ func (m StringMap) Remove(toRemove []string) StringMap { return m } +// AddedAndRemovalListsFromArray returns a list of added entries and a list of removal entries +func AddedAndRemovalListsFromArray(m []string) ([]string, []string) { + stringToRemove := []string{} + stringToAdd := []string{} + for _, key := range m { + if strings.HasSuffix(key, "-") { + stringToRemove = append(stringToRemove, key[:len(key)-1]) + } else { + stringToAdd = append(stringToAdd, key) + } + } + return stringToAdd, stringToRemove +} + // mapFromArray takes an array of strings where each item is a (key, value) pair // separated by a delimiter and returns a map where keys are mapped to their respective values. // If allowSingles is true, values without a delimiter will be added as keys pointing to empty strings diff --git a/pkg/util/parsing_helper_test.go b/pkg/util/parsing_helper_test.go index e7d135178d..e5914b505d 100644 --- a/pkg/util/parsing_helper_test.go +++ b/pkg/util/parsing_helper_test.go @@ -102,3 +102,17 @@ func TestStringMap(t *testing.T) { expectedMap := StringMap{"a1": "b1-new", "a3": "b3"} assert.DeepEqual(t, expectedMap, inputMap) } + +func TestAddedAndRemovalListFromArray(t *testing.T) { + addList, removeList := AddedAndRemovalListsFromArray([]string{"addvalue1", "remove1-", "addvalue2", "remove2-"}) + assert.DeepEqual(t, []string{"addvalue1", "addvalue2"}, addList) + assert.DeepEqual(t, []string{"remove1", "remove2"}, removeList) + + addList, removeList = AddedAndRemovalListsFromArray([]string{"remove1-"}) + assert.DeepEqual(t, []string{}, addList) + assert.DeepEqual(t, []string{"remove1"}, removeList) + + addList, removeList = AddedAndRemovalListsFromArray([]string{"addvalue1"}) + assert.DeepEqual(t, []string{"addvalue1"}, addList) + assert.DeepEqual(t, []string{}, removeList) +}