fail printing on internal obj

pull/8/head
juanvallejo 2018-05-02 11:27:45 -04:00
parent b617748f7b
commit b5f6d834fc
15 changed files with 433 additions and 42 deletions

View File

@ -155,6 +155,7 @@ go_test(
"apply_test.go",
"attach_test.go",
"clusterinfo_dump_test.go",
"cmd_printing_test.go",
"cmd_test.go",
"convert_test.go",
"cp_test.go",

View File

@ -0,0 +1,221 @@
/*
Copyright 2018 The Kubernetes 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 cmd
import (
"bytes"
"testing"
"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"
)
func TestIllegalPackageSourceCheckerThroughPrintFlags(t *testing.T) {
testCases := []struct {
name string
expectInternalObjErr bool
output string
obj runtime.Object
expectedOutput string
}{
{
name: "success printer: object containing package path beginning with forbidden prefix is rejected",
expectInternalObjErr: true,
obj: internalPod(),
},
{
name: "success printer: object containing package path with no forbidden prefix returns no error",
expectInternalObjErr: false,
obj: externalPod(),
output: "",
expectedOutput: "pod/foo succeeded\n",
},
{
name: "name printer: object containing package path beginning with forbidden prefix is rejected",
expectInternalObjErr: true,
output: "name",
obj: internalPod(),
},
{
name: "json printer: json printer is wrapped in a versioned printer - internal obj should be converted with no error",
expectInternalObjErr: false,
output: "json",
obj: internalPod(),
},
{
name: "yaml printer: yaml printer is wrapped in a versioned printer - internal obj should be converted with no error",
expectInternalObjErr: false,
output: "yaml",
obj: internalPod(),
},
}
for _, tc := range testCases {
printFlags := printers.NewPrintFlags("succeeded", legacyscheme.Scheme)
printFlags.OutputFormat = &tc.output
printer, err := printFlags.ToPrinter()
if err != nil {
t.Fatalf("unexpected error %v", err)
}
output := bytes.NewBuffer([]byte{})
err = printer.PrintObj(tc.obj, output)
if err != nil {
if !tc.expectInternalObjErr {
t.Fatalf("unexpected error %v", err)
}
if !printers.IsInternalObjectError(err) {
t.Fatalf("unexpected error - expecting internal object printer error, got %q", err)
}
continue
}
if tc.expectInternalObjErr {
t.Fatalf("expected internal object printer error, but got no error")
}
if len(tc.expectedOutput) == 0 {
continue
}
if tc.expectedOutput != output.String() {
t.Fatalf("unexpected output: expecting %q, got %q", tc.expectedOutput, output.String())
}
}
}
func TestIllegalPackageSourceCheckerDirectlyThroughPrinters(t *testing.T) {
jsonPathPrinter, err := printers.NewJSONPathPrinter("{ .metadata.name }")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
goTemplatePrinter, err := printers.NewGoTemplatePrinter([]byte("{{ .metadata.name }}"))
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
customColumns, err := printers.NewCustomColumnsPrinterFromSpec("NAME:.metadata.name", scheme.Codecs.UniversalDecoder(), true)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
testCases := []struct {
name string
expectInternalObjErr bool
printer printers.ResourcePrinter
obj runtime.Object
expectedOutput string
}{
{
name: "json printer: object containing package path beginning with forbidden prefix is rejected",
expectInternalObjErr: true,
printer: &printers.JSONPrinter{},
obj: internalPod(),
},
{
name: "yaml printer: object containing package path beginning with forbidden prefix is rejected",
expectInternalObjErr: true,
printer: &printers.YAMLPrinter{},
obj: internalPod(),
},
{
name: "jsonpath printer: object containing package path beginning with forbidden prefix is rejected",
expectInternalObjErr: true,
printer: jsonPathPrinter,
obj: internalPod(),
},
{
name: "go-template printer: object containing package path beginning with forbidden prefix is rejected",
expectInternalObjErr: true,
printer: goTemplatePrinter,
obj: internalPod(),
},
{
name: "go-template printer: object containing package path beginning with forbidden prefix is rejected",
expectInternalObjErr: true,
printer: goTemplatePrinter,
obj: internalPod(),
},
{
name: "custom-columns printer: object containing package path beginning with forbidden prefix is rejected",
expectInternalObjErr: true,
printer: customColumns,
obj: internalPod(),
},
}
for _, tc := range testCases {
output := bytes.NewBuffer([]byte{})
err := tc.printer.PrintObj(tc.obj, output)
if err != nil {
if !tc.expectInternalObjErr {
t.Fatalf("unexpected error %v", err)
}
if !printers.IsInternalObjectError(err) {
t.Fatalf("unexpected error - expecting internal object printer error, got %q", err)
}
continue
}
if tc.expectInternalObjErr {
t.Fatalf("expected internal object printer error, but got no error")
}
if len(tc.expectedOutput) == 0 {
continue
}
if tc.expectedOutput != output.String() {
t.Fatalf("unexpected output: expecting %q, got %q", tc.expectedOutput, output.String())
}
}
}
func internalPod() *api.Pod {
return &api.Pod{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "test", ResourceVersion: "10"},
Spec: api.PodSpec{
Containers: []api.Container{
{
Name: "bar",
},
},
},
Status: api.PodStatus{
Phase: api.PodRunning,
},
}
}
func externalPod() *v1.Pod {
return &v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
},
}
}

View File

@ -20,13 +20,12 @@ import (
"encoding/json"
"fmt"
"io"
"net/url"
"strings"
"github.com/golang/glog"
"github.com/spf13/cobra"
"net/url"
kapierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

View File

@ -194,7 +194,9 @@ func validateArguments(cmd *cobra.Command, filenames, args []string) error {
}
func (o *RollingUpdateOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []string) error {
o.OldName = args[0]
if len(args) > 0 {
o.OldName = args[0]
}
o.DryRun = cmdutil.GetDryRunFlag(cmd)
o.OutputFormat = cmdutil.GetFlagString(cmd, "output")
o.KeepOldName = len(args) == 1
@ -399,6 +401,7 @@ func (o *RollingUpdateOptions) Run() error {
if replicasDefaulted {
newRc.Spec.Replicas = oldRc.Spec.Replicas
}
if o.DryRun {
oldRcData := &bytes.Buffer{}
newRcData := &bytes.Buffer{}
@ -410,10 +413,10 @@ func (o *RollingUpdateOptions) Run() error {
if err != nil {
return err
}
if err := printer.PrintObj(oldRc, oldRcData); err != nil {
if err := printer.PrintObj(cmdutil.AsDefaultVersionedOrOriginal(oldRc, nil), oldRcData); err != nil {
return err
}
if err := printer.PrintObj(newRc, newRcData); err != nil {
if err := printer.PrintObj(cmdutil.AsDefaultVersionedOrOriginal(newRc, nil), newRcData); err != nil {
return err
}
}
@ -462,7 +465,7 @@ func (o *RollingUpdateOptions) Run() error {
if err != nil {
return err
}
return printer.PrintObj(newRc, o.Out)
return printer.PrintObj(cmdutil.AsDefaultVersionedOrOriginal(newRc, nil), o.Out)
}
func findNewName(args []string, oldRc *api.ReplicationController) string {

View File

@ -17,6 +17,7 @@ limitations under the License.
package util
import (
"fmt"
"net/http"
"os"
"path/filepath"
@ -32,8 +33,6 @@ import (
"k8s.io/client-go/tools/clientcmd"
"k8s.io/client-go/util/homedir"
"fmt"
"k8s.io/kubernetes/pkg/kubectl/cmd/util/transport"
)

View File

@ -86,12 +86,14 @@ filegroup(
go_test(
name = "go_default_test",
srcs = [
"flags_test.go",
"humanreadable_test.go",
"template_test.go",
],
embed = [":go_default_library"],
deps = [
"//pkg/apis/core:go_default_library",
"//vendor/k8s.io/api/core/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1beta1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",

View File

@ -151,6 +151,13 @@ type CustomColumnsPrinter struct {
}
func (s *CustomColumnsPrinter) PrintObj(obj runtime.Object, out io.Writer) error {
// we use reflect.Indirect here in order to obtain the actual value from a pointer.
// we need an actual value in order to retrieve the package path for an object.
// using reflect.Indirect indiscriminately is valid here, as all runtime.Objects are supposed to be pointers.
if internalObjectPreventer.IsForbidden(reflect.Indirect(reflect.ValueOf(obj)).Type().PkgPath()) {
return fmt.Errorf(internalObjectPrinterErr)
}
if w, found := out.(*tabwriter.Writer); !found {
w = GetNewTabWriter(out)
out = w

View File

@ -18,12 +18,25 @@ package printers
import (
"fmt"
"strings"
"github.com/spf13/cobra"
"k8s.io/apimachinery/pkg/runtime"
)
var (
internalObjectPrinterErr = "a versioned object must be passed to a printer"
// disallowedPackagePrefixes contains regular expression templates
// for object package paths that are not allowed by printers.
disallowedPackagePrefixes = []string{
"k8s.io/kubernetes/pkg/apis/",
}
)
var internalObjectPreventer = &illegalPackageSourceChecker{disallowedPackagePrefixes}
type NoCompatiblePrinterError struct {
OutputFormat *string
Options interface{}
@ -47,6 +60,14 @@ func IsNoCompatiblePrinterError(err error) bool {
return ok
}
func IsInternalObjectError(err error) bool {
if err == nil {
return false
}
return err.Error() == internalObjectPrinterErr
}
// PrintFlags composes common printer flag structs
// used across all commands, and provides a method
// of retrieving a known printer based on flag values provided.
@ -111,3 +132,22 @@ func NewPrintFlags(operation string, scheme runtime.ObjectConvertor) *PrintFlags
NamePrintFlags: NewNamePrintFlags(operation, scheme),
}
}
// illegalPackageSourceChecker compares a given
// object's package path, and determines if the
// object originates from a disallowed source.
type illegalPackageSourceChecker struct {
// disallowedPrefixes is a slice of disallowed package path
// prefixes for a given runtime.Object that we are printing.
disallowedPrefixes []string
}
func (c *illegalPackageSourceChecker) IsForbidden(pkgPath string) bool {
for _, forbiddenPrefix := range c.disallowedPrefixes {
if strings.HasPrefix(pkgPath, forbiddenPrefix) {
return true
}
}
return false
}

View File

@ -0,0 +1,66 @@
/*
Copyright 2018 The Kubernetes 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 (
"testing"
)
func TestIllegalPackageSourceChecker(t *testing.T) {
disallowedPrefixes := []string{
"foo/bar",
"k8s.io/foo/bar/vendor/k8s.io/baz/buz",
"bar/foo/baz",
}
testCases := []struct {
name string
pkgPath string
shouldBeAllowed bool
}{
{
name: "package path beginning with forbidden prefix is rejected",
pkgPath: "foo/bar/baz/buz",
shouldBeAllowed: false,
},
{
name: "package path not fully matching forbidden prefix is allowed",
pkgPath: "bar/foo",
shouldBeAllowed: true,
},
{
name: "package path containing forbidden prefix (not as prefix) is allowed",
pkgPath: "k8s.io/bar/foo/baz/etc",
shouldBeAllowed: true,
},
}
checker := &illegalPackageSourceChecker{disallowedPrefixes}
for _, tc := range testCases {
if checker.IsForbidden(tc.pkgPath) {
if tc.shouldBeAllowed {
t.Fatalf("expected package path %q to have been allowed", tc.pkgPath)
}
continue
}
if !tc.shouldBeAllowed {
t.Fatalf("expected package path %q to have been rejected", tc.pkgPath)
}
}
}

View File

@ -252,19 +252,19 @@ func testPrinter(t *testing.T, printer printers.ResourcePrinter, unmarshalFunc f
t.Errorf("Test data and unmarshaled data are not equal: %v", diff.ObjectDiff(poutput, testData))
}
obj := &api.Pod{
obj := &v1.Pod{
ObjectMeta: metav1.ObjectMeta{Name: "foo"},
}
buf.Reset()
printer.PrintObj(obj, buf)
var objOut api.Pod
var objOut v1.Pod
// Verify that given function runs without error.
err = unmarshalFunc(buf.Bytes(), &objOut)
if err != nil {
t.Fatalf("unexpected error: %#v", err)
}
// Use real decode function to undo the versioning process.
objOut = api.Pod{}
objOut = v1.Pod{}
if err := runtime.DecodeInto(s, buf.Bytes(), &objOut); err != nil {
t.Fatal(err)
}
@ -359,7 +359,7 @@ func TestTemplatePanic(t *testing.T) {
t.Fatalf("tmpl fail: %v", err)
}
buffer := &bytes.Buffer{}
err = printer.PrintObj(&api.Pod{}, buffer)
err = printer.PrintObj(&v1.Pod{}, buffer)
if err == nil {
t.Fatalf("expected that template to crash")
}
@ -374,7 +374,7 @@ func TestNamePrinter(t *testing.T) {
expect string
}{
"singleObject": {
&api.Pod{
&v1.Pod{
TypeMeta: metav1.TypeMeta{
Kind: "Pod",
},
@ -567,31 +567,25 @@ func TestPrinters(t *testing.T) {
}
jsonpathPrinter = printers.NewVersionedPrinter(jsonpathPrinter, legacyscheme.Scheme, legacyscheme.Scheme, v1.SchemeGroupVersion)
allPrinters := map[string]printers.ResourcePrinter{
"humanReadable": printers.NewHumanReadablePrinter(nil, printers.PrintOptions{
NoHeaders: true,
}),
"humanReadableHeaders": printers.NewHumanReadablePrinter(nil, printers.PrintOptions{}),
"json": &printers.JSONPrinter{},
"yaml": &printers.YAMLPrinter{},
"template": templatePrinter,
"template2": templatePrinter2,
"jsonpath": jsonpathPrinter,
genericPrinters := map[string]printers.ResourcePrinter{
"json": &printers.JSONPrinter{},
"yaml": &printers.YAMLPrinter{},
"template": templatePrinter,
"template2": templatePrinter2,
"jsonpath": jsonpathPrinter,
"name": &printers.NamePrinter{
Typer: legacyscheme.Scheme,
Decoders: []runtime.Decoder{legacyscheme.Codecs.UniversalDecoder(), unstructured.UnstructuredJSONScheme},
},
}
AddHandlers((allPrinters["humanReadable"]).(*printers.HumanReadablePrinter))
AddHandlers((allPrinters["humanReadableHeaders"]).(*printers.HumanReadablePrinter))
objects := map[string]runtime.Object{
"pod": &api.Pod{ObjectMeta: om("pod")},
"emptyPodList": &api.PodList{},
"nonEmptyPodList": &api.PodList{Items: []api.Pod{{}}},
"endpoints": &api.Endpoints{
Subsets: []api.EndpointSubset{{
Addresses: []api.EndpointAddress{{IP: "127.0.0.1"}, {IP: "localhost"}},
Ports: []api.EndpointPort{{Port: 8080}},
"pod": &v1.Pod{ObjectMeta: om("pod")},
"emptyPodList": &v1.PodList{},
"nonEmptyPodList": &v1.PodList{Items: []v1.Pod{{}}},
"endpoints": &v1.Endpoints{
Subsets: []v1.EndpointSubset{{
Addresses: []v1.EndpointAddress{{IP: "127.0.0.1"}, {IP: "localhost"}},
Ports: []v1.EndpointPort{{Port: 8080}},
}}},
}
// map of printer name to set of objects it should fail on.
@ -600,7 +594,29 @@ func TestPrinters(t *testing.T) {
"jsonpath": sets.NewString("emptyPodList", "nonEmptyPodList", "endpoints"),
}
for pName, p := range allPrinters {
for pName, p := range genericPrinters {
for oName, obj := range objects {
b := &bytes.Buffer{}
if err := p.PrintObj(obj, b); err != nil {
if set, found := expectedErrors[pName]; found && set.Has(oName) {
// expected error
continue
}
t.Errorf("printer '%v', object '%v'; error: '%v'", pName, oName, err)
}
}
}
// a humanreadable printer deals with internal-versioned objects
humanReadablePrinter := map[string]printers.ResourcePrinter{
"humanReadable": printers.NewHumanReadablePrinter(nil, printers.PrintOptions{
NoHeaders: true,
}),
"humanReadableHeaders": printers.NewHumanReadablePrinter(nil, printers.PrintOptions{}),
}
AddHandlers((humanReadablePrinter["humanReadable"]).(*printers.HumanReadablePrinter))
AddHandlers((humanReadablePrinter["humanReadableHeaders"]).(*printers.HumanReadablePrinter))
for pName, p := range humanReadablePrinter {
for oName, obj := range objects {
b := &bytes.Buffer{}
if err := p.PrintObj(obj, b); err != nil {

View File

@ -21,6 +21,7 @@ import (
"encoding/json"
"fmt"
"io"
"reflect"
"k8s.io/apimachinery/pkg/runtime"
@ -28,11 +29,17 @@ import (
)
// JSONPrinter is an implementation of ResourcePrinter which outputs an object as JSON.
type JSONPrinter struct {
}
type JSONPrinter struct{}
// PrintObj is an implementation of ResourcePrinter.PrintObj which simply writes the object to the Writer.
func (p *JSONPrinter) PrintObj(obj runtime.Object, w io.Writer) error {
// we use reflect.Indirect here in order to obtain the actual value from a pointer.
// we need an actual value in order to retrieve the package path for an object.
// using reflect.Indirect indiscriminately is valid here, as all runtime.Objects are supposed to be pointers.
if internalObjectPreventer.IsForbidden(reflect.Indirect(reflect.ValueOf(obj)).Type().PkgPath()) {
return fmt.Errorf(internalObjectPrinterErr)
}
switch obj := obj.(type) {
case *runtime.Unknown:
var buf bytes.Buffer
@ -64,6 +71,13 @@ type YAMLPrinter struct {
// PrintObj prints the data as YAML.
func (p *YAMLPrinter) PrintObj(obj runtime.Object, w io.Writer) error {
// we use reflect.Indirect here in order to obtain the actual value from a pointer.
// we need an actual value in order to retrieve the package path for an object.
// using reflect.Indirect indiscriminately is valid here, as all runtime.Objects are supposed to be pointers.
if internalObjectPreventer.IsForbidden(reflect.Indirect(reflect.ValueOf(obj)).Type().PkgPath()) {
return fmt.Errorf(internalObjectPrinterErr)
}
switch obj := obj.(type) {
case *runtime.Unknown:
data, err := yaml.JSONToYAML(obj.Raw)

View File

@ -107,11 +107,21 @@ func NewJSONPathPrinter(tmpl string) (*JSONPathPrinter, error) {
if err := j.Parse(tmpl); err != nil {
return nil, err
}
return &JSONPathPrinter{tmpl, j}, nil
return &JSONPathPrinter{
rawTemplate: tmpl,
JSONPath: j,
}, nil
}
// PrintObj formats the obj with the JSONPath Template.
func (j *JSONPathPrinter) PrintObj(obj runtime.Object, w io.Writer) error {
// we use reflect.Indirect here in order to obtain the actual value from a pointer.
// we need an actual value in order to retrieve the package path for an object.
// using reflect.Indirect indiscriminately is valid here, as all runtime.Objects are supposed to be pointers.
if internalObjectPreventer.IsForbidden(reflect.Indirect(reflect.ValueOf(obj)).Type().PkgPath()) {
return fmt.Errorf(internalObjectPrinterErr)
}
var queryObj interface{} = obj
if meta.IsListType(obj) {
data, err := json.Marshal(obj)

View File

@ -19,6 +19,7 @@ package printers
import (
"fmt"
"io"
"reflect"
"strings"
"k8s.io/apimachinery/pkg/api/meta"
@ -45,6 +46,13 @@ type NamePrinter struct {
// PrintObj is an implementation of ResourcePrinter.PrintObj which decodes the object
// and print "resource/name" pair. If the object is a List, print all items in it.
func (p *NamePrinter) PrintObj(obj runtime.Object, w io.Writer) error {
// we use reflect.Indirect here in order to obtain the actual value from a pointer.
// using reflect.Indirect indiscriminately is valid here, as all runtime.Objects are supposed to be pointers.
// we need an actual value in order to retrieve the package path for an object.
if internalObjectPreventer.IsForbidden(reflect.Indirect(reflect.ValueOf(obj)).Type().PkgPath()) {
return fmt.Errorf(internalObjectPrinterErr)
}
if meta.IsListType(obj) {
items, err := meta.ExtractList(obj)
if err != nil {

View File

@ -21,6 +21,7 @@ import (
"encoding/json"
"fmt"
"io"
"reflect"
"text/template"
"k8s.io/apimachinery/pkg/runtime"
@ -59,6 +60,10 @@ func (p *GoTemplatePrinter) AllowMissingKeys(allow bool) {
// PrintObj formats the obj with the Go Template.
func (p *GoTemplatePrinter) PrintObj(obj runtime.Object, w io.Writer) error {
if internalObjectPreventer.IsForbidden(reflect.Indirect(reflect.ValueOf(obj)).Type().PkgPath()) {
return fmt.Errorf(internalObjectPrinterErr)
}
var data []byte
var err error
data, err = json.Marshal(obj)

View File

@ -21,8 +21,8 @@ import (
"strings"
"testing"
"k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/runtime"
api "k8s.io/kubernetes/pkg/apis/core"
)
func TestTemplate(t *testing.T) {
@ -33,8 +33,8 @@ func TestTemplate(t *testing.T) {
expectErr func(error) (string, bool)
}{
"support base64 decoding of secret data": {
template: "{{ .Data.username | base64decode }}",
obj: &api.Secret{
template: "{{ .data.username | base64decode }}",
obj: &v1.Secret{
Data: map[string][]byte{
"username": []byte("hunter"),
},
@ -42,7 +42,7 @@ func TestTemplate(t *testing.T) {
expectOut: "hunter",
},
"test error path for base64 decoding": {
template: "{{ .Data.username | base64decode }}",
template: "{{ .data.username | base64decode }}",
obj: &badlyMarshaledSecret{},
expectErr: func(err error) (string, bool) {
matched := strings.Contains(err.Error(), "base64 decode")
@ -89,9 +89,9 @@ func TestTemplate(t *testing.T) {
}
type badlyMarshaledSecret struct {
api.Secret
v1.Secret
}
func (a badlyMarshaledSecret) MarshalJSON() ([]byte, error) {
return []byte(`{"apiVersion":"v1","Data":{"username":"--THIS IS NOT BASE64--"},"kind":"Secret"}`), nil
return []byte(`{"apiVersion":"v1","data":{"username":"--THIS IS NOT BASE64--"},"kind":"Secret"}`), nil
}