From 7f11585972dd787a2b9d42f3457ec388cd58b15b Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Thu, 7 May 2015 13:53:43 -0700 Subject: [PATCH] Fix validation by moving it into the resource builder. Also always print an error for unknown field. --- pkg/api/validation/schema.go | 3 ++- pkg/kubectl/cmd/create.go | 4 +--- pkg/kubectl/cmd/rollingupdate.go | 5 ++++ pkg/kubectl/cmd/update.go | 4 +--- pkg/kubectl/resource/builder.go | 13 ++++++++++- pkg/kubectl/resource/visitor.go | 39 +++++++++++++++++++++++++++++--- 6 files changed, 57 insertions(+), 11 deletions(-) diff --git a/pkg/api/validation/schema.go b/pkg/api/validation/schema.go index 19ab40f4128..879957478d3 100644 --- a/pkg/api/validation/schema.go +++ b/pkg/api/validation/schema.go @@ -104,9 +104,10 @@ func (s *SwaggerSchema) ValidateObject(obj interface{}, apiVersion, fieldName, t for key, value := range fields { details, ok := properties[key] if !ok { + glog.Infof("unknown field: %s", key) // Some properties can be missing because of // https://github.com/GoogleCloudPlatform/kubernetes/issues/6842. - glog.V(2).Infof("couldn't find properties for %s", key) + glog.Info("this may be a false alarm, see https://github.com/GoogleCloudPlatform/kubernetes/issues/6842") continue } if details.Type == nil && details.Ref == nil { diff --git a/pkg/kubectl/cmd/create.go b/pkg/kubectl/cmd/create.go index 043b8ff2a94..064cab8d9c9 100644 --- a/pkg/kubectl/cmd/create.go +++ b/pkg/kubectl/cmd/create.go @@ -79,6 +79,7 @@ func RunCreate(f *cmdutil.Factory, out io.Writer, filenames util.StringList) err mapper, typer := f.Object() r := resource.NewBuilder(mapper, typer, f.ClientMapperForCommand()). + Schema(schema). ContinueOnError(). NamespaceParam(cmdNamespace).RequireNamespace(). FilenameParam(filenames...). @@ -95,9 +96,6 @@ func RunCreate(f *cmdutil.Factory, out io.Writer, filenames util.StringList) err if err != nil { return err } - if err := schema.ValidateBytes(data); err != nil { - return err - } obj, err := resource.NewHelper(info.Client, info.Mapping).Create(info.Namespace, true, data) if err != nil { return err diff --git a/pkg/kubectl/cmd/rollingupdate.go b/pkg/kubectl/cmd/rollingupdate.go index 3e161f29fed..e5401942fda 100644 --- a/pkg/kubectl/cmd/rollingupdate.go +++ b/pkg/kubectl/cmd/rollingupdate.go @@ -143,7 +143,12 @@ func RunRollingUpdate(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, arg var newRc *api.ReplicationController if len(filename) != 0 { + schema, err := f.Validator() + if err != nil { + return err + } obj, err := resource.NewBuilder(mapper, typer, f.ClientMapperForCommand()). + Schema(schema). NamespaceParam(cmdNamespace).RequireNamespace(). FilenameParam(filename). Do(). diff --git a/pkg/kubectl/cmd/update.go b/pkg/kubectl/cmd/update.go index 13e6dd3d847..7898ddce192 100644 --- a/pkg/kubectl/cmd/update.go +++ b/pkg/kubectl/cmd/update.go @@ -93,6 +93,7 @@ func RunUpdate(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []str mapper, typer := f.Object() r := resource.NewBuilder(mapper, typer, f.ClientMapperForCommand()). + Schema(schema). ContinueOnError(). NamespaceParam(cmdNamespace).RequireNamespace(). FilenameParam(filenames...). @@ -108,9 +109,6 @@ func RunUpdate(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []str if err != nil { return err } - if err := schema.ValidateBytes(data); err != nil { - return err - } obj, err := resource.NewHelper(info.Client, info.Mapping).Update(info.Namespace, info.Name, true, data) if err != nil { return err diff --git a/pkg/kubectl/resource/builder.go b/pkg/kubectl/resource/builder.go index e03fb10eb0f..8ddfcd6eab9 100644 --- a/pkg/kubectl/resource/builder.go +++ b/pkg/kubectl/resource/builder.go @@ -24,6 +24,7 @@ import ( "strings" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" @@ -62,6 +63,8 @@ type Builder struct { singleResourceType bool continueOnError bool + + schema validation.Schema } type resourceTuple struct { @@ -77,6 +80,11 @@ func NewBuilder(mapper meta.RESTMapper, typer runtime.ObjectTyper, clientMapper } } +func (b *Builder) Schema(schema validation.Schema) *Builder { + b.schema = schema + return b +} + // Filename is parameters passed via a filename argument which may be URLs, the "-" argument indicating // STDIN, or paths to files or directories. If ContinueOnError() is set prior to this method being called, // objects on the path that are unrecognized will be ignored (but logged at V(2)). @@ -105,6 +113,7 @@ func (b *Builder) URL(urls ...*url.URL) *Builder { b.paths = append(b.paths, &URLVisitor{ Mapper: b.mapper, URL: u, + Schema: b.schema, }) } return b @@ -123,7 +132,7 @@ func (b *Builder) Stdin() *Builder { // will be ignored (but logged at V(2)). func (b *Builder) Stream(r io.Reader, name string) *Builder { b.stream = true - b.paths = append(b.paths, NewStreamVisitor(r, b.mapper, name, b.continueOnError)) + b.paths = append(b.paths, NewStreamVisitor(r, b.mapper, b.schema, name, b.continueOnError)) return b } @@ -150,12 +159,14 @@ func (b *Builder) Path(paths ...string) *Builder { Extensions: []string{".json", ".yaml", ".yml"}, Recursive: false, IgnoreErrors: b.continueOnError, + Schema: b.schema, } } else { visitor = &PathVisitor{ Mapper: b.mapper, Path: p, IgnoreErrors: b.continueOnError, + Schema: b.schema, } } b.paths = append(b.paths, visitor) diff --git a/pkg/kubectl/resource/visitor.go b/pkg/kubectl/resource/visitor.go index 6f2a272ce30..dce54c8d156 100644 --- a/pkg/kubectl/resource/visitor.go +++ b/pkg/kubectl/resource/visitor.go @@ -29,6 +29,7 @@ import ( "github.com/golang/glog" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/GoogleCloudPlatform/kubernetes/pkg/util/errors" "github.com/GoogleCloudPlatform/kubernetes/pkg/util/yaml" @@ -180,6 +181,20 @@ func (l EagerVisitorList) Visit(fn VisitorFunc) error { return errors.NewAggregate(errs) } +func ValidateSchema(data []byte, schema validation.Schema) error { + if schema == nil { + return nil + } + data, err := yaml.ToJSON(data) + if err != nil { + return fmt.Errorf("error converting to YAML: %v", err) + } + if err := schema.ValidateBytes(data); err != nil { + return fmt.Errorf("error validating data: %v", err) + } + return nil +} + // PathVisitor visits a given path and returns an object representing the file // at that path. type PathVisitor struct { @@ -188,6 +203,8 @@ type PathVisitor struct { Path string // Whether to ignore files that are not recognized as API objects IgnoreErrors bool + // Schema for validation + Schema validation.Schema } func (v *PathVisitor) Visit(fn VisitorFunc) error { @@ -195,6 +212,9 @@ func (v *PathVisitor) Visit(fn VisitorFunc) error { if err != nil { return fmt.Errorf("unable to read %q: %v", v.Path, err) } + if err := ValidateSchema(data, v.Schema); err != nil { + return err + } info, err := v.Mapper.InfoForData(data, v.Path) if err != nil { if v.IgnoreErrors { @@ -218,6 +238,8 @@ type DirectoryVisitor struct { Extensions []string // Whether to ignore files that are not recognized as API objects IgnoreErrors bool + // Schema for validation + Schema validation.Schema } func (v *DirectoryVisitor) ignoreFile(path string) bool { @@ -253,6 +275,9 @@ func (v *DirectoryVisitor) Visit(fn VisitorFunc) error { if err != nil { return fmt.Errorf("unable to read %q: %v", path, err) } + if err := ValidateSchema(data, v.Schema); err != nil { + return err + } info, err := v.Mapper.InfoForData(data, path) if err != nil { if v.IgnoreErrors { @@ -269,7 +294,8 @@ func (v *DirectoryVisitor) Visit(fn VisitorFunc) error { // an info object representing the downloaded object. type URLVisitor struct { *Mapper - URL *url.URL + URL *url.URL + Schema validation.Schema } func (v *URLVisitor) Visit(fn VisitorFunc) error { @@ -285,6 +311,9 @@ func (v *URLVisitor) Visit(fn VisitorFunc) error { if err != nil { return fmt.Errorf("unable to read URL %q: %v\n", v.URL, err) } + if err := ValidateSchema(data, v.Schema); err != nil { + return err + } info, err := v.Mapper.InfoForData(data, v.URL.String()) if err != nil { return err @@ -379,6 +408,7 @@ type StreamVisitor struct { Source string IgnoreErrors bool + Schema validation.Schema } // NewStreamVisitor creates a visitor that will return resources that were encoded into the provided @@ -386,8 +416,8 @@ type StreamVisitor struct { // empty stream is treated as an error for now. // TODO: convert ignoreErrors into a func(data, error, count) bool that consumers can use to decide // what to do with ignored errors. -func NewStreamVisitor(r io.Reader, mapper *Mapper, source string, ignoreErrors bool) Visitor { - return &StreamVisitor{r, mapper, source, ignoreErrors} +func NewStreamVisitor(r io.Reader, mapper *Mapper, schema validation.Schema, source string, ignoreErrors bool) Visitor { + return &StreamVisitor{r, mapper, source, ignoreErrors, schema} } // Visit implements Visitor over a stream. @@ -405,6 +435,9 @@ func (v *StreamVisitor) Visit(fn VisitorFunc) error { if len(ext.RawJSON) == 0 || bytes.Equal(ext.RawJSON, []byte("null")) { continue } + if err := ValidateSchema(ext.RawJSON, v.Schema); err != nil { + return err + } info, err := v.InfoForData(ext.RawJSON, v.Source) if err != nil { if v.IgnoreErrors {