Skip to content

Commit

Permalink
Merge pull request kubernetes#63928 from deads2k/cli-61-apiversion-pr…
Browse files Browse the repository at this point in the history
…otection

Automatic merge from submit-queue (batch tested with PRs 63920, 63716, 63928, 60553, 63946). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

add protection for missing apiversion so we never serialize a bad object

we need the json and yaml printers to fail if they are going to serialize a thing that is missing apiversion and kind information.  This adds a simple check for it.

@kubernetes/sig-cli-maintainers 
/assign @juanvallejo 
/assign @soltysh 

```release-note
NONE
```
  • Loading branch information
Kubernetes Submit Queue authored May 18, 2018
2 parents 23d3b6f + 420dd9b commit 062f6b5
Show file tree
Hide file tree
Showing 59 changed files with 342 additions and 413 deletions.
4 changes: 2 additions & 2 deletions pkg/kubectl/cmd/annotate.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ import (
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/json"

"k8s.io/kubernetes/pkg/api/legacyscheme"
"k8s.io/kubernetes/pkg/kubectl"
"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
"k8s.io/kubernetes/pkg/kubectl/genericclioptions"
"k8s.io/kubernetes/pkg/kubectl/genericclioptions/resource"
"k8s.io/kubernetes/pkg/kubectl/scheme"
"k8s.io/kubernetes/pkg/kubectl/util/i18n"
"k8s.io/kubernetes/pkg/printers"
)
Expand Down Expand Up @@ -111,7 +111,7 @@ var (

func NewAnnotateOptions(ioStreams genericclioptions.IOStreams) *AnnotateOptions {
return &AnnotateOptions{
PrintFlags: printers.NewPrintFlags("annotated", legacyscheme.Scheme),
PrintFlags: printers.NewPrintFlags("annotated").WithTypeSetter(scheme.Scheme),

RecordFlags: genericclioptions.NewRecordFlags(),
Recorder: genericclioptions.NoopRecorder{},
Expand Down
3 changes: 1 addition & 2 deletions pkg/kubectl/cmd/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ import (
"k8s.io/client-go/dynamic"
scaleclient "k8s.io/client-go/scale"
oapi "k8s.io/kube-openapi/pkg/util/proto"
"k8s.io/kubernetes/pkg/api/legacyscheme"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
"k8s.io/kubernetes/pkg/kubectl"
Expand Down Expand Up @@ -132,7 +131,7 @@ func NewApplyOptions(ioStreams genericclioptions.IOStreams) *ApplyOptions {
return &ApplyOptions{
RecordFlags: genericclioptions.NewRecordFlags(),
DeleteFlags: NewDeleteFlags("that contains the configuration to apply"),
PrintFlags: printers.NewPrintFlags("created", legacyscheme.Scheme),
PrintFlags: printers.NewPrintFlags("created").WithTypeSetter(scheme.Scheme),

Overwrite: true,
OpenApiPatch: true,
Expand Down
4 changes: 2 additions & 2 deletions pkg/kubectl/cmd/apply_set_last_applied.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ import (
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/kubernetes/pkg/api/legacyscheme"
"k8s.io/kubernetes/pkg/kubectl"
"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
"k8s.io/kubernetes/pkg/kubectl/cmd/util/editor"
"k8s.io/kubernetes/pkg/kubectl/genericclioptions"
"k8s.io/kubernetes/pkg/kubectl/genericclioptions/resource"
"k8s.io/kubernetes/pkg/kubectl/scheme"
"k8s.io/kubernetes/pkg/kubectl/util/i18n"
"k8s.io/kubernetes/pkg/printers"
)
Expand Down Expand Up @@ -83,7 +83,7 @@ var (

func NewSetLastAppliedOptions(ioStreams genericclioptions.IOStreams) *SetLastAppliedOptions {
return &SetLastAppliedOptions{
PrintFlags: printers.NewPrintFlags("configured", legacyscheme.Scheme),
PrintFlags: printers.NewPrintFlags("configured").WithTypeSetter(scheme.Scheme),
IOStreams: ioStreams,
}
}
Expand Down
1 change: 1 addition & 0 deletions pkg/kubectl/cmd/auth/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ go_library(
"//pkg/kubectl/cmd/util:go_default_library",
"//pkg/kubectl/genericclioptions:go_default_library",
"//pkg/kubectl/genericclioptions/resource:go_default_library",
"//pkg/kubectl/scheme:go_default_library",
"//pkg/printers:go_default_library",
"//pkg/registry/rbac/reconciliation:go_default_library",
"//vendor/github.com/golang/glog:go_default_library",
Expand Down
3 changes: 2 additions & 1 deletion pkg/kubectl/cmd/auth/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
"k8s.io/kubernetes/pkg/kubectl/genericclioptions"
"k8s.io/kubernetes/pkg/kubectl/genericclioptions/resource"
"k8s.io/kubernetes/pkg/kubectl/scheme"
"k8s.io/kubernetes/pkg/printers"
"k8s.io/kubernetes/pkg/registry/rbac/reconciliation"
)
Expand Down Expand Up @@ -63,7 +64,7 @@ var (
func NewReconcileOptions(ioStreams genericclioptions.IOStreams) *ReconcileOptions {
return &ReconcileOptions{
FilenameOptions: &resource.FilenameOptions{},
PrintFlags: printers.NewPrintFlags("reconciled", legacyscheme.Scheme),
PrintFlags: printers.NewPrintFlags("reconciled").WithTypeSetter(scheme.Scheme),
IOStreams: ioStreams,
}
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/kubectl/cmd/autoscale.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
"k8s.io/kubernetes/pkg/kubectl/genericclioptions"
"k8s.io/kubernetes/pkg/kubectl/genericclioptions/resource"
"k8s.io/kubernetes/pkg/kubectl/scheme"
"k8s.io/kubernetes/pkg/kubectl/util/i18n"
"k8s.io/kubernetes/pkg/printers"
)
Expand Down Expand Up @@ -82,7 +83,7 @@ type AutoscaleOptions struct {

func NewAutoscaleOptions(ioStreams genericclioptions.IOStreams) *AutoscaleOptions {
return &AutoscaleOptions{
PrintFlags: printers.NewPrintFlags("autoscaled", legacyscheme.Scheme),
PrintFlags: printers.NewPrintFlags("autoscaled").WithTypeSetter(scheme.Scheme),
FilenameOptions: &resource.FilenameOptions{},
RecordFlags: genericclioptions.NewRecordFlags(),
Recorder: genericclioptions.NoopRecorder{},
Expand Down
5 changes: 3 additions & 2 deletions pkg/kubectl/cmd/certificates.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
"k8s.io/kubernetes/pkg/kubectl/genericclioptions"
"k8s.io/kubernetes/pkg/kubectl/genericclioptions/resource"
"k8s.io/kubernetes/pkg/kubectl/scheme"
"k8s.io/kubernetes/pkg/kubectl/util/i18n"
"k8s.io/kubernetes/pkg/printers"

Expand Down Expand Up @@ -99,7 +100,7 @@ func (o *CertificateOptions) Validate() error {

func NewCmdCertificateApprove(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *cobra.Command {
options := CertificateOptions{
PrintFlags: printers.NewPrintFlags("approved", legacyscheme.Scheme),
PrintFlags: printers.NewPrintFlags("approved").WithTypeSetter(scheme.Scheme),
IOStreams: ioStreams,
}
cmd := &cobra.Command{
Expand Down Expand Up @@ -156,7 +157,7 @@ func (o *CertificateOptions) RunCertificateApprove(force bool) error {

func NewCmdCertificateDeny(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *cobra.Command {
options := CertificateOptions{
PrintFlags: printers.NewPrintFlags("denied", legacyscheme.Scheme),
PrintFlags: printers.NewPrintFlags("denied").WithTypeSetter(scheme.Scheme),
IOStreams: ioStreams,
}
cmd := &cobra.Command{
Expand Down
4 changes: 2 additions & 2 deletions pkg/kubectl/cmd/clusterinfo_dump.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ import (
"github.com/spf13/cobra"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/kubernetes/pkg/api/legacyscheme"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
"k8s.io/kubernetes/pkg/kubectl/genericclioptions"
"k8s.io/kubernetes/pkg/kubectl/polymorphichelpers"
"k8s.io/kubernetes/pkg/kubectl/scheme"
"k8s.io/kubernetes/pkg/kubectl/util/i18n"
"k8s.io/kubernetes/pkg/printers"
)
Expand All @@ -57,7 +57,7 @@ type ClusterInfoDumpOptions struct {
// NewCmdCreateSecret groups subcommands to create various types of secrets
func NewCmdClusterInfoDump(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *cobra.Command {
o := &ClusterInfoDumpOptions{
PrintFlags: printers.NewPrintFlags("", legacyscheme.Scheme),
PrintFlags: printers.NewPrintFlags("").WithTypeSetter(scheme.Scheme),

IOStreams: ioStreams,
}
Expand Down
21 changes: 16 additions & 5 deletions pkg/kubectl/cmd/cmd_printing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/kubernetes/pkg/api/legacyscheme"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/kubectl/scheme"
"k8s.io/kubernetes/pkg/printers"
Expand Down Expand Up @@ -56,21 +55,33 @@ func TestIllegalPackageSourceCheckerThroughPrintFlags(t *testing.T) {
obj: internalPod(),
},
{
name: "json printer: json printer is wrapped in a versioned printer - internal obj should be converted with no error",
expectInternalObjErr: false,
name: "json printer: object containing package path beginning with forbidden prefix is rejected",
expectInternalObjErr: true,
output: "json",
obj: internalPod(),
},
{
name: "yaml printer: yaml printer is wrapped in a versioned printer - internal obj should be converted with no error",
name: "json printer: object containing package path with no forbidden prefix returns no error",
expectInternalObjErr: false,
obj: externalPod(),
output: "json",
},
{
name: "yaml printer: object containing package path beginning with forbidden prefix is rejected",
expectInternalObjErr: true,
output: "yaml",
obj: internalPod(),
},
{
name: "yaml printer: object containing package path with no forbidden prefix returns no error",
expectInternalObjErr: false,
obj: externalPod(),
output: "yaml",
},
}

for _, tc := range testCases {
printFlags := printers.NewPrintFlags("succeeded", legacyscheme.Scheme)
printFlags := printers.NewPrintFlags("succeeded").WithTypeSetter(scheme.Scheme)
printFlags.OutputFormat = &tc.output

printer, err := printFlags.ToPrinter()
Expand Down
16 changes: 10 additions & 6 deletions pkg/kubectl/cmd/config/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ type kubectlConfigPrintFlags struct {
NamePrintFlags *printers.NamePrintFlags
TemplateFlags *printers.KubeTemplatePrintFlags

TypeSetter *printers.TypeSetterPrinter

OutputFormat *string
}

Expand All @@ -45,15 +47,15 @@ func (f *kubectlConfigPrintFlags) ToPrinter() (printers.ResourcePrinter, error)
}

if p, err := f.JSONYamlPrintFlags.ToPrinter(outputFormat); !printers.IsNoCompatiblePrinterError(err) {
return p, err
return f.TypeSetter.WrapToPrinter(p, err)
}

if p, err := f.NamePrintFlags.ToPrinter(outputFormat); !printers.IsNoCompatiblePrinterError(err) {
return p, err
return f.TypeSetter.WrapToPrinter(p, err)
}

if p, err := f.TemplateFlags.ToPrinter(outputFormat); !printers.IsNoCompatiblePrinterError(err) {
return p, err
return f.TypeSetter.WrapToPrinter(p, err)
}

return nil, printers.NoCompatiblePrinterError{Options: f}
Expand All @@ -75,14 +77,16 @@ func (f *kubectlConfigPrintFlags) WithDefaultOutput(output string) *kubectlConfi
return f
}

func newKubeConfigPrintFlags(scheme runtime.ObjectConvertor) *kubectlConfigPrintFlags {
func newKubeConfigPrintFlags(scheme runtime.ObjectTyper) *kubectlConfigPrintFlags {
outputFormat := ""

return &kubectlConfigPrintFlags{
OutputFormat: &outputFormat,

JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(scheme),
NamePrintFlags: printers.NewNamePrintFlags("", scheme),
JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(),
NamePrintFlags: printers.NewNamePrintFlags(""),
TemplateFlags: printers.NewKubeTemplatePrintFlags(),

TypeSetter: printers.NewTypeSetter(scheme),
}
}
2 changes: 1 addition & 1 deletion pkg/kubectl/cmd/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ type ConvertOptions struct {

func NewConvertOptions(ioStreams genericclioptions.IOStreams) *ConvertOptions {
return &ConvertOptions{
PrintFlags: printers.NewPrintFlags("converted", scheme.Scheme).WithDefaultOutput("yaml"),
PrintFlags: printers.NewPrintFlags("converted").WithTypeSetter(scheme.Scheme).WithDefaultOutput("yaml"),
local: true,
IOStreams: ioStreams,
}
Expand Down
12 changes: 7 additions & 5 deletions pkg/kubectl/cmd/create/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ import (
"runtime"
"strings"

"github.com/golang/glog"
"github.com/spf13/cobra"

"github.com/golang/glog"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
kruntime "k8s.io/apimachinery/pkg/runtime"
Expand All @@ -40,6 +40,7 @@ import (
"k8s.io/kubernetes/pkg/kubectl/genericclioptions"
"k8s.io/kubernetes/pkg/kubectl/genericclioptions/resource"
"k8s.io/kubernetes/pkg/kubectl/util/i18n"
"k8s.io/kubernetes/pkg/printers"
)

type CreateOptions struct {
Expand Down Expand Up @@ -350,7 +351,7 @@ type CreateSubcommandOptions struct {
Mapper meta.RESTMapper
DynamicClient dynamic.Interface

PrintObj func(obj kruntime.Object) error
PrintObj printers.ResourcePrinterFunc

genericclioptions.IOStreams
}
Expand Down Expand Up @@ -381,8 +382,8 @@ func (o *CreateSubcommandOptions) Complete(f cmdutil.Factory, cmd *cobra.Command
return err
}

o.PrintObj = func(obj kruntime.Object) error {
return printer.PrintObj(obj, o.Out)
o.PrintObj = func(obj kruntime.Object, out io.Writer) error {
return printer.PrintObj(obj, out)
}

o.Namespace, o.EnforceNamespace, err = f.DefaultNamespace()
Expand Down Expand Up @@ -426,6 +427,7 @@ func (o *CreateSubcommandOptions) Run() error {
}

asUnstructured := &unstructured.Unstructured{}

if err := legacyscheme.Scheme.Convert(obj, asUnstructured, nil); err != nil {
return err
}
Expand All @@ -445,5 +447,5 @@ func (o *CreateSubcommandOptions) Run() error {
}
}

return o.PrintObj(obj)
return o.PrintObj(obj, o.Out)
}
6 changes: 5 additions & 1 deletion pkg/kubectl/cmd/create/create_clusterrole.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/spf13/cobra"

rbacv1 "k8s.io/api/rbac/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
"k8s.io/kubernetes/pkg/kubectl/genericclioptions"
Expand Down Expand Up @@ -156,7 +157,10 @@ func (c *CreateClusterRoleOptions) Validate() error {
}

func (c *CreateClusterRoleOptions) RunCreateRole() error {
clusterRole := &rbacv1.ClusterRole{}
clusterRole := &rbacv1.ClusterRole{
// this is ok because we know exactly how we want to be serialized
TypeMeta: metav1.TypeMeta{APIVersion: rbacv1.SchemeGroupVersion.String(), Kind: "ClusterRole"},
}
clusterRole.Name = c.Name
rules, err := generateResourcePolicyRules(c.Mapper, c.Verbs, c.Resources, c.ResourceNames, c.NonResourceURLs)
if err != nil {
Expand Down
6 changes: 5 additions & 1 deletion pkg/kubectl/cmd/create/create_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

rbacv1 "k8s.io/api/rbac/v1"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/sets"
Expand Down Expand Up @@ -299,7 +300,10 @@ func (o *CreateRoleOptions) validateResource() error {
}

func (o *CreateRoleOptions) RunCreateRole() error {
role := &rbacv1.Role{}
role := &rbacv1.Role{
// this is ok because we know exactly how we want to be serialized
TypeMeta: metav1.TypeMeta{APIVersion: rbacv1.SchemeGroupVersion.String(), Kind: "Role"},
}
role.Name = o.Name
rules, err := generateResourcePolicyRules(o.Mapper, o.Verbs, o.Resources, o.ResourceNames, []string{})
if err != nil {
Expand Down
Loading

0 comments on commit 062f6b5

Please sign in to comment.