Server Side Field Validation #2
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,7 +23,8 @@ import ( | |
| // Prune removes object fields in obj which are not specified in s. It skips TypeMeta and ObjectMeta fields | ||
| // if XEmbeddedResource is set to true, or for the root if isResourceRoot=true, i.e. it does not | ||
| // prune unknown metadata fields. | ||
| // It returns the set of fields that it prunes. | ||
| func Prune(obj interface{}, s *structuralschema.Structural, isResourceRoot bool) map[string]bool { | ||
|
There was a problem hiding this comment. Does this have to be a map, or can it be a list? What are the string? The type is not explaining a lot about what it is. There was a problem hiding this comment. see suggestion above about making this return a list of |
||
| if isResourceRoot { | ||
| if s == nil { | ||
| s = &structuralschema.Structural{} | ||
|
|
@@ -34,7 +35,7 @@ func Prune(obj interface{}, s *structuralschema.Structural, isResourceRoot bool) | |
| s = &clone | ||
| } | ||
| } | ||
| return prune(obj, s) | ||
| } | ||
|
|
||
| var metaFields = map[string]bool{ | ||
|
|
@@ -43,51 +44,71 @@ var metaFields = map[string]bool{ | |
| "metadata": true, | ||
| } | ||
|
|
||
| func prune(x interface{}, s *structuralschema.Structural) map[string]bool { | ||
| if s != nil && s.XPreserveUnknownFields { | ||
| return skipPrune(x, s) | ||
| } | ||
|
|
||
| pruning := map[string]bool{} | ||
| switch x := x.(type) { | ||
| case map[string]interface{}: | ||
| if s == nil { | ||
| for k := range x { | ||
| if !metaFields[k] { | ||
| pruning[k] = true | ||
| } | ||
| delete(x, k) | ||
| } | ||
| return pruning | ||
| } | ||
| for k, v := range x { | ||
| if s.XEmbeddedResource && metaFields[k] { | ||
| continue | ||
| } | ||
| prop, ok := s.Properties[k] | ||
| if ok { | ||
| pruned := prune(v, &prop) | ||
| for k, b := range pruned { | ||
| pruning[k] = b | ||
| } | ||
| } else if s.AdditionalProperties != nil { | ||
| pruned := prune(v, s.AdditionalProperties.Structural) | ||
| for k, b := range pruned { | ||
| pruning[k] = b | ||
| } | ||
| } else { | ||
| if !metaFields[k] { | ||
| pruning[k] = true | ||
| } | ||
| delete(x, k) | ||
| } | ||
| } | ||
| case []interface{}: | ||
| if s == nil { | ||
| for _, v := range x { | ||
| pruned := prune(v, nil) | ||
| for k, b := range pruned { | ||
| pruning[k] = b | ||
| } | ||
| } | ||
| return pruning | ||
| } | ||
| for _, v := range x { | ||
| pruned := prune(v, s.Items) | ||
| for k, b := range pruned { | ||
| pruning[k] = b | ||
| } | ||
| } | ||
| default: | ||
| // scalars, do nothing | ||
| } | ||
| return pruning | ||
| } | ||
|
|
||
| func skipPrune(x interface{}, s *structuralschema.Structural) map[string]bool { | ||
| pruning := map[string]bool{} | ||
| if s == nil { | ||
| return pruning | ||
| } | ||
|
|
||
| switch x := x.(type) { | ||
|
|
@@ -97,16 +118,26 @@ func skipPrune(x interface{}, s *structuralschema.Structural) { | |
| continue | ||
| } | ||
| if prop, ok := s.Properties[k]; ok { | ||
| pruned := prune(v, &prop) | ||
| for k, b := range pruned { | ||
| pruning[k] = b | ||
| } | ||
| } else if s.AdditionalProperties != nil { | ||
| pruned := prune(v, s.AdditionalProperties.Structural) | ||
| for k, b := range pruned { | ||
| pruning[k] = b | ||
| } | ||
| } | ||
| } | ||
| case []interface{}: | ||
| for _, v := range x { | ||
| skipPruned := skipPrune(v, s.Items) | ||
| for k, b := range skipPruned { | ||
| pruning[k] = b | ||
| } | ||
| } | ||
| default: | ||
| // scalars, do nothing | ||
| } | ||
| return pruning | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -544,6 +544,16 @@ type CreateOptions struct { | |
| // as defined by https://golang.org/pkg/unicode/#IsPrint. | ||
| // +optional | ||
| FieldManager string `json:"fieldManager,omitempty" protobuf:"bytes,3,name=fieldManager"` | ||
|
|
||
| // fieldValidation determines how the server should respond to | ||
kevindelgado marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // unknown/duplicate fields. | ||
| // TODO: Do we still need a protobuf tag if protobuf is not supported? | ||
kevindelgado marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // Valid values are: | ||
| // - Ignore: ignore's unknown/duplicate fields | ||
| // - Strict: fail the request on unknown/duplicate fields | ||
| // - Warn: respond with a warning, but successfully serve the request. | ||
|
Comment on lines
+552
to
+554
There was a problem hiding this comment. So have we decided that warn is to expensive to be the default (and that Ignore is unnecessary?) There was a problem hiding this comment. I'd suggest ordering these in increasing severity: Ignore And document what the default is on servers that support the fieldValidation option (I expected we could default to Warn) There was a problem hiding this comment. reordered the values. Waiting on another round of feedback before defaulting to warn to confirm that is what we want to do |
||
| // +optional | ||
| FieldValidation string `json:"fieldValidation,omitempty"` | ||
| } | ||
|
|
||
| // +k8s:conversion-gen:explicit-from=net/url.Values | ||
|
|
@@ -577,6 +587,16 @@ type PatchOptions struct { | |
| // types (JsonPatch, MergePatch, StrategicMergePatch). | ||
| // +optional | ||
| FieldManager string `json:"fieldManager,omitempty" protobuf:"bytes,3,name=fieldManager"` | ||
|
|
||
| // fieldValidation determines how the server should respond to | ||
| // unknown/duplicate fields. | ||
| // TODO: Do we still need a protobuf tag if protobuf is not supported? | ||
| // Valid values are: | ||
| // - Ignore: ignore's unknown/duplicate fields | ||
kevindelgado marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // - Strict: fail the request on unknown/duplicate fields | ||
| // - Warn: respond with a warning, but successfully serve the request. | ||
| // +optional | ||
| FieldValidation string `json:"fieldValidation,omitempty"` | ||
| } | ||
|
|
||
| // ApplyOptions may be provided when applying an API object. | ||
|
|
@@ -632,6 +652,16 @@ type UpdateOptions struct { | |
| // as defined by https://golang.org/pkg/unicode/#IsPrint. | ||
| // +optional | ||
| FieldManager string `json:"fieldManager,omitempty" protobuf:"bytes,2,name=fieldManager"` | ||
|
|
||
| // fieldValidation determines how the server should respond to | ||
| // unknown/duplicate fields. | ||
| // TODO: Do we still need a protobuf tag if protobuf is not supported? | ||
| // Valid values are: | ||
| // - Ignore: ignore's unknown/duplicate fields | ||
| // - Strict: fail the request on unknown/duplicate fields | ||
| // - Warn: respond with a warning, but successfully serve the request. | ||
| // +optional | ||
| FieldValidation string `json:"fieldValidation,omitempty"` | ||
| } | ||
|
|
||
| // Preconditions must be fulfilled before an operation (update, delete, etc.) is carried out. | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not completely sure about the name yet, what about ...
StrictValidation, or bothStrictFieldsValidation?