From 0254441c5994de1e2571deae6d2bbac8e349fbb0 Mon Sep 17 00:00:00 2001 From: Daisy Guo Date: Mon, 23 Dec 2019 18:00:01 +0800 Subject: [PATCH] improve 'source apiserver update --resource' to follow the common update semantics --- docs/cmd/kn_source_apiserver_create.md | 2 +- docs/cmd/kn_source_apiserver_update.md | 2 +- .../sources/v1alpha1/apiserver_client.go | 44 ++++++ pkg/kn/commands/source/apiserver/create.go | 19 +-- .../source/apiserver/create_flag_test.go | 105 ------------- pkg/kn/commands/source/apiserver/flags.go | 47 ++++-- .../commands/source/apiserver/flags_test.go | 148 ++++++++++++++++++ pkg/kn/commands/source/apiserver/update.go | 12 +- 8 files changed, 246 insertions(+), 133 deletions(-) delete mode 100644 pkg/kn/commands/source/apiserver/create_flag_test.go create mode 100644 pkg/kn/commands/source/apiserver/flags_test.go diff --git a/docs/cmd/kn_source_apiserver_create.md b/docs/cmd/kn_source_apiserver_create.md index 3ff858d7b9..28df5a8f7c 100644 --- a/docs/cmd/kn_source_apiserver_create.md +++ b/docs/cmd/kn_source_apiserver_create.md @@ -26,7 +26,7 @@ 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. + --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. --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..9face71071 100644 --- a/docs/cmd/kn_source_apiserver_update.md +++ b/docs/cmd/kn_source_apiserver_update.md @@ -26,7 +26,7 @@ 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. + --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. --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/eventing/sources/v1alpha1/apiserver_client.go b/pkg/eventing/sources/v1alpha1/apiserver_client.go index 3a29ffd20a..7684261b16 100644 --- a/pkg/eventing/sources/v1alpha1/apiserver_client.go +++ b/pkg/eventing/sources/v1alpha1/apiserver_client.go @@ -15,6 +15,9 @@ 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" @@ -22,6 +25,10 @@ import ( duckv1beta1 "knative.dev/pkg/apis/duck/v1beta1" ) +const ( + apiVersionSplitChar = ":" +) + // KnAPIServerSourcesClient interface for working with ApiServer sources type KnAPIServerSourcesClient interface { @@ -156,6 +163,43 @@ 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 diff --git a/pkg/kn/commands/source/apiserver/create.go b/pkg/kn/commands/source/apiserver/create.go index 797fd1054c..58946dd543 100644 --- a/pkg/kn/commands/source/apiserver/create.go +++ b/pkg/kn/commands/source/apiserver/create.go @@ -64,19 +64,20 @@ 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 } + for _, k := range resources { + b.AddResource(k.ApiVersion, k.Kind, k.IsController) + } - 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 2b9559ec01..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/sources/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 bf5973ae5e..1eac376704 100644 --- a/pkg/kn/commands/source/apiserver/flags.go +++ b/pkg/kn/commands/source/apiserver/flags.go @@ -41,27 +41,44 @@ type APIServerSourceUpdateFlags struct { } type resourceSpec struct { - kind string - apiVersion string - isController bool + 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) { - var resourceList []v1alpha1.ApiServerResource +// 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 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 +} + +// getAPIServerResourceArray is to return an array of ApiServerResource from a string. A sample is Event:v1:true,Pod:v2:false +func (f *APIServerSourceUpdateFlags) getUpdateAPIServerResourceArray() ([]resourceSpec, []resourceSpec, error) { + var added []resourceSpec + var removed []resourceSpec + for _, r := range f.Resources { + if strings.HasSuffix(r, "-") { + resourceSpec, err := getValidResource(r[:len(r)-1]) + if err != nil { + return nil, nil, err + } + removed = append(removed, *resourceSpec) + } else { + resourceSpec, err := getValidResource(r) + if err != nil { + return nil, nil, err + } + added = append(added, *resourceSpec) } - resourceList = append(resourceList, resourceRef) } - return &resourceList, nil + return added, removed, nil } func getValidResource(resource string) (*resourceSpec, error) { @@ -80,7 +97,7 @@ func getValidResource(resource string) (*resourceSpec, error) { return nil, fmt.Errorf("cannot parse controller flage in resource specification %s", resource) } } - return &resourceSpec{apiVersion: version, kind: kind, isController: isController}, nil + return &resourceSpec{Kind: kind, ApiVersion: version, IsController: isController}, nil } //Add is to set parameters @@ -95,9 +112,9 @@ 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, + []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.`) } 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..8d790f57e7 --- /dev/null +++ b/pkg/kn/commands/source/apiserver/flags_test.go @@ -0,0 +1,148 @@ +// 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" +) + +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 := []resourceSpec{{ + Kind: "Service", + ApiVersion: "serving.knative.dev/v1alpha1", + IsController: 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 = []resourceSpec{{ + Kind: "Service", + ApiVersion: "serving.knative.dev/v1alpha1", + IsController: 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 = []resourceSpec{{ + Kind: "Service", + ApiVersion: "v1", + IsController: 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 := []resourceSpec{{ + Kind: "Event", + ApiVersion: "v1", + IsController: true, + }, { + Kind: "Pod", + ApiVersion: "v2", + IsController: 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 = []resourceSpec{{ + Kind: "Event", + ApiVersion: "v1", + IsController: false, + }, { + Kind: "Pod", + ApiVersion: "v1", + IsController: false, + }} + assert.DeepEqual(t, wanted, created) + }) +} + +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 := []resourceSpec{{ + Kind: "Event", + ApiVersion: "v1", + IsController: true, + }} + removewanted := []resourceSpec{{ + Kind: "Pod", + ApiVersion: "v2", + IsController: 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 = []resourceSpec{{ + Kind: "Event", + ApiVersion: "v1", + IsController: false, + }} + addwanted = []resourceSpec{{ + Kind: "Pod", + ApiVersion: "v1", + IsController: false, + }} + assert.DeepEqual(t, added, addwanted) + assert.DeepEqual(t, removed, removewanted) + }) +} diff --git a/pkg/kn/commands/source/apiserver/update.go b/pkg/kn/commands/source/apiserver/update.go index e51ba091ad..4060e0b987 100644 --- a/pkg/kn/commands/source/apiserver/update.go +++ b/pkg/kn/commands/source/apiserver/update.go @@ -74,11 +74,19 @@ func NewAPIServerUpdateCommand(p *commands.KnParams) *cobra.Command { } if cmd.Flags().Changed("resource") { - resources, err := apiServerUpdateFlags.GetAPIServerResourceArray() + added, removed, err := apiServerUpdateFlags.getUpdateAPIServerResourceArray() if err != nil { return err } - b.Resources(*resources) + for _, k := range removed { + _, err = b.RemoveResource(k.ApiVersion, k.Kind, k.IsController) + if err != nil { + return err + } + } + for _, k := range added { + b.AddResource(k.ApiVersion, k.Kind, k.IsController) + } } if cmd.Flags().Changed("sink") {