From 6d0fa5d6f10a165b426fcbebe07649458fe64279 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20Hu=C3=9F?= Date: Thu, 30 May 2019 21:50:08 +0200 Subject: [PATCH 1/2] fix(serving): Update GVK on objects within a ResourcePrinter. This is an alternate solution to #134 for fixing #133. In this commit the update of the GVK coordinates are moved to the resource printer. --- pkg/kn/commands/revision_describe.go | 7 +--- pkg/kn/commands/revision_get.go | 8 ++-- pkg/kn/commands/revision_get_test.go | 2 +- pkg/kn/commands/service_describe.go | 8 +--- pkg/kn/commands/service_describe_test.go | 2 +- pkg/kn/commands/service_get.go | 5 --- pkg/kn/commands/service_get_flags.go | 4 +- pkg/kn/commands/service_get_test.go | 2 +- pkg/kn/commands/service_update_test.go | 6 +-- pkg/printers/gvk_update_writer.go | 40 ++++++++++++++++++++ pkg/serving/schema_handling.go | 48 ++++++++++++++++++++++++ pkg/serving/schema_handling_test.go | 43 +++++++++++++++++++++ 12 files changed, 146 insertions(+), 29 deletions(-) create mode 100644 pkg/printers/gvk_update_writer.go create mode 100644 pkg/serving/schema_handling.go create mode 100644 pkg/serving/schema_handling_test.go diff --git a/pkg/kn/commands/revision_describe.go b/pkg/kn/commands/revision_describe.go index 68c595a0a2..66c8e68ff0 100644 --- a/pkg/kn/commands/revision_describe.go +++ b/pkg/kn/commands/revision_describe.go @@ -16,10 +16,10 @@ package commands import ( "errors" + "github.com/knative/client/pkg/printers" "github.com/spf13/cobra" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/cli-runtime/pkg/genericclioptions" ) @@ -51,10 +51,7 @@ func NewRevisionDescribeCommand(p *KnParams) *cobra.Command { if err != nil { return err } - revision.GetObjectKind().SetGroupVersionKind(schema.GroupVersionKind{ - Group: "knative.dev", - Version: "v1alpha1", - Kind: "Revision"}) + printer = printers.NewGvkUpdatePrinter(printer) err = printer.PrintObj(revision, cmd.OutOrStdout()) if err != nil { return err diff --git a/pkg/kn/commands/revision_get.go b/pkg/kn/commands/revision_get.go index 33d9f8b965..707a34094d 100644 --- a/pkg/kn/commands/revision_get.go +++ b/pkg/kn/commands/revision_get.go @@ -16,10 +16,10 @@ package commands import ( "fmt" + "github.com/knative/client/pkg/printers" "github.com/spf13/cobra" "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" ) // NewRevisionGetCommand represents 'kn revision get' command @@ -46,14 +46,12 @@ func NewRevisionGetCommand(p *KnParams) *cobra.Command { fmt.Fprintf(cmd.OutOrStdout(), "No resources found.\n") return nil } - revision.GetObjectKind().SetGroupVersionKind(schema.GroupVersionKind{ - Group: "knative.dev", - Version: "v1alpha1", - Kind: "revision"}) + printer, err := revisionGetFlags.ToPrinter() if err != nil { return err } + printer = printers.NewGvkUpdatePrinter(printer) err = printer.PrintObj(revision, cmd.OutOrStdout()) if err != nil { return err diff --git a/pkg/kn/commands/revision_get_test.go b/pkg/kn/commands/revision_get_test.go index 9ebab3835b..9eb0bea143 100644 --- a/pkg/kn/commands/revision_get_test.go +++ b/pkg/kn/commands/revision_get_test.go @@ -86,7 +86,7 @@ func createMockRevisionWithParams(name, svcName string) *v1alpha1.Revision { revision := &v1alpha1.Revision{ TypeMeta: metav1.TypeMeta{ Kind: "Revision", - APIVersion: "knative.dev/v1alpha1", + APIVersion: "serving.knative.dev/v1alpha1", }, ObjectMeta: metav1.ObjectMeta{ Name: name, diff --git a/pkg/kn/commands/service_describe.go b/pkg/kn/commands/service_describe.go index 7052ddad43..a5d91d7044 100644 --- a/pkg/kn/commands/service_describe.go +++ b/pkg/kn/commands/service_describe.go @@ -16,10 +16,9 @@ package commands import ( "errors" - + "github.com/knative/client/pkg/printers" "github.com/spf13/cobra" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/cli-runtime/pkg/genericclioptions" ) @@ -51,10 +50,7 @@ func NewServiceDescribeCommand(p *KnParams) *cobra.Command { if err != nil { return err } - describeService.GetObjectKind().SetGroupVersionKind(schema.GroupVersionKind{ - Group: "knative.dev", - Version: "v1alpha1", - Kind: "Service"}) + printer = printers.NewGvkUpdatePrinter(printer) err = printer.PrintObj(describeService, cmd.OutOrStdout()) if err != nil { return err diff --git a/pkg/kn/commands/service_describe_test.go b/pkg/kn/commands/service_describe_test.go index 4cac4a46c4..2b58c11666 100644 --- a/pkg/kn/commands/service_describe_test.go +++ b/pkg/kn/commands/service_describe_test.go @@ -62,7 +62,7 @@ func TestServiceDescribeDefaultOutput(t *testing.T) { expectedService := v1alpha1.Service{ TypeMeta: metav1.TypeMeta{ Kind: "Service", - APIVersion: "knative.dev/v1alpha1", + APIVersion: "serving.knative.dev/v1alpha1", }, ObjectMeta: metav1.ObjectMeta{ Name: "foo", diff --git a/pkg/kn/commands/service_get.go b/pkg/kn/commands/service_get.go index 1f22129649..8ecfb1bdb4 100644 --- a/pkg/kn/commands/service_get.go +++ b/pkg/kn/commands/service_get.go @@ -19,7 +19,6 @@ import ( "github.com/spf13/cobra" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" ) // NewServiceGetCommand represents 'kn service get' command @@ -46,10 +45,6 @@ func NewServiceGetCommand(p *KnParams) *cobra.Command { fmt.Fprintf(cmd.OutOrStdout(), "No resources found.\n") return nil } - service.GetObjectKind().SetGroupVersionKind(schema.GroupVersionKind{ - Group: "knative.dev", - Version: "v1alpha1", - Kind: "Service"}) printer, err := serviceGetFlags.ToPrinter() if err != nil { diff --git a/pkg/kn/commands/service_get_flags.go b/pkg/kn/commands/service_get_flags.go index 40980e388a..3473844976 100644 --- a/pkg/kn/commands/service_get_flags.go +++ b/pkg/kn/commands/service_get_flags.go @@ -52,14 +52,14 @@ func (f *ServiceGetFlags) ToPrinter() (hprinters.ResourcePrinter, error) { if err != nil { return nil, err } - return p, nil + return hprinters.NewGvkUpdatePrinter(p), nil } // if no flags specified, use the table printing p, err := f.HumanReadableFlags.ToPrinter() if err != nil { return nil, err } - return p, nil + return hprinters.NewGvkUpdatePrinter(p), nil } // AddFlags receives a *cobra.Command reference and binds diff --git a/pkg/kn/commands/service_get_test.go b/pkg/kn/commands/service_get_test.go index 17ec61e8cf..2f46aa8393 100644 --- a/pkg/kn/commands/service_get_test.go +++ b/pkg/kn/commands/service_get_test.go @@ -94,7 +94,7 @@ func createMockServiceWithParams(name, domain string, generation int64) *v1alpha service := &v1alpha1.Service{ TypeMeta: metav1.TypeMeta{ Kind: "Service", - APIVersion: "knative.dev/v1alpha1", + APIVersion: "serving.knative.dev/v1alpha1", }, ObjectMeta: metav1.ObjectMeta{ Name: name, diff --git a/pkg/kn/commands/service_update_test.go b/pkg/kn/commands/service_update_test.go index 6e3869fc0a..605a675377 100644 --- a/pkg/kn/commands/service_update_test.go +++ b/pkg/kn/commands/service_update_test.go @@ -74,7 +74,7 @@ func TestServiceUpdateImage(t *testing.T) { orig := &v1alpha1.Service{ TypeMeta: metav1.TypeMeta{ Kind: "Service", - APIVersion: "knative.dev/v1alpha1", + APIVersion: "serving.knative.dev/v1alpha1", }, ObjectMeta: metav1.ObjectMeta{ Name: "foo", @@ -120,7 +120,7 @@ func TestServiceUpdateEnv(t *testing.T) { orig := &v1alpha1.Service{ TypeMeta: metav1.TypeMeta{ Kind: "Service", - APIVersion: "knative.dev/v1alpha1", + APIVersion: "serving.knative.dev/v1alpha1", }, ObjectMeta: metav1.ObjectMeta{ Name: "foo", @@ -289,7 +289,7 @@ func createMockServiceWithResources(t *testing.T, requestCPU, requestMemory, lim service := &v1alpha1.Service{ TypeMeta: metav1.TypeMeta{ Kind: "Service", - APIVersion: "knative.dev/v1alpha1", + APIVersion: "serving.knative.dev/v1alpha1", }, ObjectMeta: metav1.ObjectMeta{ Name: "foo", diff --git a/pkg/printers/gvk_update_writer.go b/pkg/printers/gvk_update_writer.go new file mode 100644 index 0000000000..96d132f922 --- /dev/null +++ b/pkg/printers/gvk_update_writer.go @@ -0,0 +1,40 @@ +// 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 printers + +import ( + "github.com/knative/client/pkg/serving" + "github.com/knative/serving/pkg/apis/serving/v1alpha1" + "io" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/cli-runtime/pkg/genericclioptions/printers" +) + +type wrappedGvkUpdatePrinter struct { + printer printers.ResourcePrinter +} + +// Wrapper printer for updating an object with GVK just before printing +func NewGvkUpdatePrinter(delegate printers.ResourcePrinter) printers.ResourcePrinter { + return wrappedGvkUpdatePrinter{delegate} +} + +func (gvkPrinter wrappedGvkUpdatePrinter) PrintObj(obj runtime.Object, writer io.Writer) error { + // TODO: How to deal with multiple versions which needs to be supported soon ? + if err := serving.UpdateGroupVersionKind(obj, v1alpha1.SchemeGroupVersion); err != nil { + return err + } + return gvkPrinter.printer.PrintObj(obj, writer) +} \ No newline at end of file diff --git a/pkg/serving/schema_handling.go b/pkg/serving/schema_handling.go new file mode 100644 index 0000000000..72601254d4 --- /dev/null +++ b/pkg/serving/schema_handling.go @@ -0,0 +1,48 @@ +// 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 serving + +import ( + "errors" + "fmt" + + "github.com/knative/serving/pkg/client/clientset/versioned/scheme" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +// Update the GVK on the given object, based on the GVK registered in into the serving scheme +// for the given GroupVersion +func UpdateGroupVersionKind(obj runtime.Object, gv schema.GroupVersion) error { + gvk, err := getGroupVersionKind(obj, gv) + if err != nil { + return err + } + obj.GetObjectKind().SetGroupVersionKind(*gvk) + return nil +} + +func getGroupVersionKind(obj runtime.Object, gv schema.GroupVersion) (*schema.GroupVersionKind, error) { + gvks, _, err := scheme.Scheme.ObjectKinds(obj) + if err != nil { + return nil, err + } + for _, gvk := range gvks { + if gvk.GroupVersion() == gv { + return &gvk, nil + } + } + return nil, errors.New(fmt.Sprintf("no group version %s registered in %s", gv, scheme.Scheme.Name())) +} diff --git a/pkg/serving/schema_handling_test.go b/pkg/serving/schema_handling_test.go new file mode 100644 index 0000000000..65529510de --- /dev/null +++ b/pkg/serving/schema_handling_test.go @@ -0,0 +1,43 @@ +// 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 serving + +import ( + "github.com/knative/serving/pkg/apis/serving/v1alpha1" + "k8s.io/apimachinery/pkg/runtime/schema" + "testing" +) + +func TestGVKUpdate(t *testing.T) { + service := v1alpha1.Service{} + err := UpdateGroupVersionKind(&service, v1alpha1.SchemeGroupVersion) + if err != nil { + t.Fatalf("cannot update GVK to a service %v", err) + } + if service.Kind != "Service" { + t.Fatalf("wrong kind '%s'", service.Kind) + } + if service.APIVersion != v1alpha1.SchemeGroupVersion.Group+"/"+v1alpha1.SchemeGroupVersion.Version { + t.Fatalf("wrong version '%s'", service.APIVersion) + } +} + +func TestGVKUpdateNegative(t *testing.T) { + service := v1alpha1.Service{} + err := UpdateGroupVersionKind(&service, schema.GroupVersion{Group: "bla", Version: "blub"}) + if err == nil { + t.Fatal("expect an error for an unregistered group version") + } +} From 6c9f728dd87b6f4d6bd7f3111098604d749a1917 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20Hu=C3=9F?= Date: Fri, 31 May 2019 18:22:53 +0200 Subject: [PATCH 2/2] chore(revision get): Moved GVK update to flag's ToPrinter method --- pkg/kn/commands/revision_get.go | 3 --- pkg/kn/commands/revision_get_flags.go | 7 ++++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/pkg/kn/commands/revision_get.go b/pkg/kn/commands/revision_get.go index 707a34094d..b2d69bb65a 100644 --- a/pkg/kn/commands/revision_get.go +++ b/pkg/kn/commands/revision_get.go @@ -16,8 +16,6 @@ package commands import ( "fmt" - "github.com/knative/client/pkg/printers" - "github.com/spf13/cobra" "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -51,7 +49,6 @@ func NewRevisionGetCommand(p *KnParams) *cobra.Command { if err != nil { return err } - printer = printers.NewGvkUpdatePrinter(printer) err = printer.PrintObj(revision, cmd.OutOrStdout()) if err != nil { return err diff --git a/pkg/kn/commands/revision_get_flags.go b/pkg/kn/commands/revision_get_flags.go index ae7ddfab04..2d44a70707 100644 --- a/pkg/kn/commands/revision_get_flags.go +++ b/pkg/kn/commands/revision_get_flags.go @@ -16,7 +16,8 @@ package commands import ( hprinters "github.com/knative/client/pkg/printers" - serving "github.com/knative/serving/pkg/apis/serving" + "github.com/knative/serving/pkg/apis/serving" + servingv1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1" "github.com/spf13/cobra" metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1" @@ -47,14 +48,14 @@ func (f *RevisionGetFlags) ToPrinter() (hprinters.ResourcePrinter, error) { if err != nil { return nil, err } - return p, nil + return hprinters.NewGvkUpdatePrinter(p), nil } // if no flags specified, use the table printing p, err := f.HumanReadableFlags.ToPrinter() if err != nil { return nil, err } - return p, nil + return hprinters.NewGvkUpdatePrinter(p), nil } // AddFlags receives a *cobra.Command reference and binds